On Wed, 21 Nov 2012, Michael W. Bombardieri wrote:
> Hi Tech,
>
> I have a small msdosfs patch...
>
> Error cases are handled inconsistently in msdosfs_mount().
> Some cases use the construct "if (error) goto error;",
> others just return error immediately.
> The following patch removes the goto label and makes the
> function return error consistently.
>
> Does this look OK, or is this change not worth the bother?
IMO it is preferable to have all of the exit conditions as a goto - it is then
completely obvious as to how much state needs to be cleaned up on exit (if
any).
> Index: msdosfs_vfsops.c
> ===================================================================
> RCS file: /cvs/src/sys/msdosfs/msdosfs_vfsops.c,v
> retrieving revision 1.62
> diff -u -r1.62 msdosfs_vfsops.c
> --- msdosfs_vfsops.c 10 Sep 2012 11:10:59 -0000 1.62
> +++ msdosfs_vfsops.c 11 Oct 2012 09:18:01 -0000
> @@ -168,12 +168,12 @@
> */
> error = copyinstr(args.fspec, fspec, sizeof(fspec), NULL);
> if (error)
> - goto error;
> + return (error);
> disk_map(fspec, fspec, MNAMELEN, DM_OPENBLCK);
>
> NDINIT(ndp, LOOKUP, FOLLOW, UIO_SYSSPACE, fspec, p);
> if ((error = namei(ndp)) != 0)
> - goto error;
> + return (error);
>
> devvp = ndp->ni_vp;
>
> @@ -232,7 +232,7 @@
> else {
> if ((error = msdosfs_root(mp, &rvp)) != 0) {
> msdosfs_unmount(mp, MNT_FORCE, p);
> - goto error;
> + return (error);
> }
> pmp->pm_flags |= findwin95(VTODE(rvp))
> ? MSDOSFSMNT_LONGNAME
> @@ -256,8 +256,6 @@
>
> error_devvp:
> vrele(devvp);
> -
> -error:
> return (error);
> }
--
"Reason is not automatic. Those who deny it cannot be conquered by it.
Do not count on them. Leave them alone." -- Ayn Rand
