Date: Wed, 20 Sep 1995 16:01:26 -0700 (MST) From: Terry Lambert <terry@lambert.org> To: mpp@mpp.minn.net (Mike Pritchard) Cc: terry@lambert.org, nate@rocky.sri.MT.net, davidg@root.com, hackers@freefall.freebsd.org Subject: Re: Coding style ( was Re: why is this not a bug in namei?) Message-ID: <199509202301.QAA01780@phaeton.artisoft.com> In-Reply-To: <199509202158.QAA12896@mpp.minn.net> from "Mike Pritchard" at Sep 20, 95 04:58:44 pm
next in thread | previous in thread | raw e-mail | index | archive | help
> My previous example was very simplistic. Having to lock multiple objects
> during a function wasn't not uncommon.
> E.g.
>
> XXX_LOCK();
> ...
> if (error_condition) {
> XXX_UNLOCK();
> return (EWHATEVER);
> }
> ...
> XXX_UNLOCK();
> ...
> if (error_condition2)
> return (EHWATEVER);
> ...
> YYY_LOCK();
> ...
> if (error_condition2) {
> YYY_UNLOCK();
> return (EWHATEVER);
> }
> ...
> YYY_UNLOCK();
> return (0);
>
> In the above example, with gotos you would need 4 exit points: 1 for
> an exit without any error. And one each for each of the 3 error
> conditions, since they all have different requirements. 2 of them have
> different locks, and 1 has no locks at all.
I think we are getting confused between function single entry/exit and
lock metastate single threading.
XXX_LOCK();
...
if (error_condition) {
err = EWHATEVER;
goto error;
}
...
XXX_UNLOCK();
...
if (error_condition2) {
err = EHWATEVER;
goto error;
}
...
YYY_LOCK();
...
if (error_condition2) {
err = EWHATEVER;
} else {
...
}
YYY_UNLOCK();
error:
return( err);
Again, this one leads to case simplification to simply:
XXX_LOCK();
...
if (!error_condition) {
...
}
XXX_UNLOCK();
if (!error_condition) {
...
if (error_condition2) {
err = EHWATEVER;
} else {
...
YYY_LOCK();
...
if (error_condition2) {
err = EWHATEVER;
} else {
...
}
YYY_UNLOCK();
}
} else {
err = EWHATEVER;
}
return( err);
I think for a "goto" example, you'll need a cyclic graph. And then
I will defend the existance of the goto. 8-).
Cyclic graphs exist in the code; they just aren't very common.
Directory reaversal in ufs_vnops.c:ufs_rename() is one good example,
though they are hidden by a pretty much bogus division of lookup()
into lookup() and relookup() (namei/lookup/relookup are simply some
of the worst code in the file system code path).
The current cycles in vfs_syscalls.c:mount() (update/nonupdate cases)
and vfs_syscalls.c:open(), rename(), etc. are caused by a flag omission
and a 6 line addition being missing from in 4 cases, and a bad job of
code overloading of not-very-common code in 3 others.
It's very hard to find an example that I would mung up with goto's
in the current code (I've added 4 "bogus_namei:" labels with one
goto each in 4 different functions, each XXX commented for future
cleanup when namei() is fixed. They are not intended as permanent
features). Certainly, I object to the "update:" and "out:" labels in
mount() much more.
Regards,
Terry Lambert
terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199509202301.QAA01780>