Re: Good Use of GOTO
Jim Brown wrote:
>
> c.k.lester wrote:
> >
> > After hearing from both sides, I've come to the conclusion that the goto
> > being implemented is unnecessary. We are getting loop constructs that
> > handle most of what GOTO would be used for, aren't we?
> >
> > continue, exit, retry, etc...
> >
> > These all lend themselves to maintenance, program flow, structure, while a
> > GOTO does not.
> >
> > Can somebody show me a case (with sample code) where a GOTO would be
> > significantly better than using another construct? Also, please explain
> > why Euphoria would need GOTO if we add PCRE to the interpreter.
>
> How would you implement/rewrite the following functions (from Linux kernel
> 2.6.11
> source) ? Even if you had the full repretiore of eu 4.0 keywords? (which,
> admittedly,
> the author of this code did not have)
>
> static struct dentry *vfat_lookup(struct inode *dir, struct dentry *dentry,
> struct nameidata *nd)
> {
> int res;
> struct vfat_slot_info sinfo;
> struct inode *inode;
> struct dentry *alias;
> struct buffer_head *bh = NULL
> struct msdos_dir_entry *de;
> int table;
>
> lock_kernel();
> table = (MSDOS_SB(dir->i_sb)->options.name_check == 's') ? 2 : 0;
> dentry->d_op = &vfat_dentry_ops[table];
>
> inode = NULL;
> res = vfat_find(dir, &dentry->d_name, &sinfo, &bh, &de);
> if (res < 0) {
> table++;
> goto error;
> }
> inode = fat_build_inode(dir->i_sb, de, sinfo.i_pos, &res);
> brelse(bh); if (res) {
> unlock_kernel();
> return ERR_PTR(res);
> }
> alias = d_find_alias(inode);
> if (alias) {
> if (d_invalidate(alias) == 0)
> dput(alias);
> else {
> iput(inode);
> unlock_kernel();
> return alias;
> }
>
> }
> error:
> unlock_kernel();
> dentry->d_op = &vfat_dentry_ops[table];
> dentry->d_time = dentry->d_parent->d_inode->i_version;
> dentry = d_splice_alias(inode, dentry);
> if (dentry) {
> dentry->d_op = &vfat_dentry_ops[table];
> dentry->d_time = dentry->d_parent->d_inode->i_version;
> }
> return dentry;
> }
>
> static int vfat_create(struct inode *dir, struct dentry *dentry, int mode,
> struct nameidata *nd)
> {
> struct super_block *sb = dir->i_sb;
> struct inode *inode = NULL;
> struct buffer_head *bh = NULL;
> struct msdos_dir_entry *de;
> struct vfat_slot_info sinfo;
> int res;
>
> lock_kernel();
> res = vfat_add_entry(dir, &dentry->d_name, 0, &sinfo, &bh, &de);
> if (res < 0)
> goto out;
> inode = fat_build_inode(sb, de, sinfo.i_pos, &res);
> brelse(bh);
> if (!inode)
> goto out;
> res = 0;
> inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
> mark_inode_dirty(inode);
> inode->i_version++;
> dir->i_version++;
> dentry->d_time = dentry->d_parent->d_inode->i_version;
> d_instantiate(dentry, inode);
> out:
> unlock_kernel();
> return res;
> }
>
> static int vfat_build_slots(struct inode *dir, const unsigned char *name,
> int len, struct msdos_dir_slot *ds,
> int *slots, int is_dir)
> {
> struct msdos_sb_info *sbi = MSDOS_SB(dir->i_sb);
> struct fat_mount_options *opts = &sbi->options;
> struct msdos_dir_slot *ps;
> struct msdos_dir_entry *de;
> unsigned long page;
> unsigned char cksum, lcase;
> unsigned char msdos_name[MSDOS_NAME];
> wchar_t *uname;
> int res, slot, ulen, usize, i;
> loff_t offset;
>
> *slots = 0;
> res = vfat_valid_longname(name, len);
> if (res)
> return res;
>
> page = __get_free_page(GFP_KERNEL);
> if (!page)
> return -ENOMEM;
>
> uname = (wchar_t *)page;
> res = xlate_to_uni(name, len, (unsigned char *)uname, &ulen, &usize,
> opts->unicode_xlate, opts->utf8, sbi->nls_io);
> if (res < 0)
> goto out_free;
>
> res = vfat_is_used_badchars(uname, ulen);
> if (res < 0)
> goto out_free;
>
> res = vfat_create_shortname(dir, sbi->nls_disk, uname, ulen,
> msdos_name, &lcase);
> if (res < 0)
> goto out_free;
> else if (res == 1) {
> de = (struct msdos_dir_entry *)ds;
> res = 0;
> goto shortname;
> }
>
> /* build the entry of long file name */
> *slots = usize / 13;
> for (cksum = i = 0; i < 11; i++)
> cksum = (((cksum&1)<<7)|((cksum&0xfe)>>1)) + msdos_name[i];
>
> for (ps = ds, slot = *slots; slot > 0; slot--, ps++) {
> ps->id = slot;
> ps->attr = ATTR_EXT;
> ps->reserved = 0;
> ps->alias_checksum = cksum;
> ps->start = 0;
> offset = (slot - 1) * 13;
> fatwchar_to16(ps->name0_4, uname + offset, 5);
> fatwchar_to16(ps->name5_10, uname + offset + 5, 6);
> fatwchar_to16(ps->name11_12, uname + offset + 11, 2);
> }
> ds[0].id |= 0x40;
> de = (struct msdos_dir_entry *)ps;
>
> shortname:
> /* build the entry of 8.3 alias name */
> (*slots)++;
> memcpy(de->name, msdos_name, MSDOS_NAME);
> de->attr = is_dir ? ATTR_DIR : ATTR_ARCH;
> de->lcase = lcase;
> de->adate = de->cdate = de->date = 0;
> de->ctime = de->time = 0;
> de->ctime_ms = 0;
> de->start = 0;
> de->starthi = 0;
> de->size = 0;
>
> out_free:
> free_page(page);
> return res;
> }
My official vote was "abstain", but I took this as a personal challenge. Plus,
it sounded fun.
Now, this is actually two functions and I've only done one so far. Note: I don't
think most arguments apply to this first function, especially "don't repeat
yourself" or whether the thing is maintainable or not. Heck, I think that this
piece of C code could probably use goto far better than it has done, but the
challenge wasn't to rewrite this piece of code in C.
-- let's assume that all structs, pointers to structs, and references are
actually sequences.
function vfat_lookup(sequence dir_p, sequence dentry_p, sequence nd_p)
integer res
sequence sinfo
sequence inode_p
sequence alias_p
sequence bh_p
sequence de_p
integer table
object tmp
lock_kernel()
tmp = msdos_sb(dir_p[I_SB])
tmp = tmp[OPTIONS[NAME_CHECK]]
if tmp = 's' then
table = 2
else
table = 0
end if
dentry_p[D_OP] = vfat_dentry_ops[table]
inode_p = {}
res = vfat_find(dir_p, dentry_p[D_NAME], sinfo, bh_p, de_p)
if res < 0 then
table++
-- error!
else
while 1 do -- I don't usually code this way...
inode_p = fat_build_inode(dir_p[I_SB], de_p, sinfo[I_POS], res)
brelse(bh_p) if res then
unlock_kernel()
return err_ptr(res)
end if
alias = d_find_alias(inode_p)
if length(alias) > 0 then
if d_invalidate(alias) = 0 then
dput(alias)
else
iput(inode_p)
unlock_kernel()
return alias
end if
end if
exit -- error!
end while
end if
-- error!
unlock_kernel()
dentry_p[D_OP] = vfat_dentry_ops[table]
dentry_p[D_TIME] = dentry[D_PARENT][D_INODE][I_VERSION]
dentry_p = d_splice_alias(inode_p, dentry_p)
if length(dentry_p) > 0 then
dentry_p[D_OP] = vfat_dentry_ops[table]
dentry_p[D_TIME] = dentry_p[D_PARENT][D_INODE][I_VERSION]
end if
return dentry_p
end function
(once again wishing euforum had a preview mode so I don't look too foolish)
--
A complex system that works is invariably found to have evolved from a simple
system that works.
--John Gall's 15th law of Systemantics.
"Premature optimization is the root of all evil in programming."
--C.A.R. Hoare
j.
|
Not Categorized, Please Help
|
|
