Date: Mon, 1 Apr 2002 20:28:46 -0800 (PST) From: Matthew Dillon <dillon@apollo.backplane.com> To: "M. Warner Losh" <imp@village.org> Cc: jake@locore.ca, dillon@FreeBSD.ORG, cvs-committers@FreeBSD.ORG, cvs-all@FreeBSD.ORG Subject: Re: cvs commit: src/sys/i386/i386 critical.c src/sys/i386/include cpufunc.h critical.h src/sys/i386/isa apic_vector.s icu_vector.s src/sys/kern kern_fork.c kern_proc.c kern_switch.c src/sys/alpha/alpha critical.c src/sys/alpha/include cpufunc.h ... Message-ID: <200204020428.g324SkE14265@apollo.backplane.com> References: <200204012351.g31NpO890339@freefall.freebsd.org> <20020401.175136.106024419.imp@village.org> <20020401201130.K207@locore.ca> <20020401.181628.15900667.imp@village.org>
next in thread | previous in thread | raw e-mail | index | archive | help
:In message: <20020401201130.K207@locore.ca>
: Jake Burkholder <jake@locore.ca> writes:
:: Apparently, On Mon, Apr 01, 2002 at 05:51:36PM -0700,
:: M. Warner Losh said words to the effect of;
::
:: > In message: <200204012351.g31NpO890339@freefall.freebsd.org>
:: > Matt Dillon <dillon@FreeBSD.org> writes:
:: > : Note: In general, developers should not gratuitously move declarations out
:: > : of sub-blocks. They are where they are for reasons of structure, grouping,
:: > : readability, compiler-localizability, and to avoid developer-introduced bugs
:: > : similar to several found in recent years in the VFS and VM code.
:: >
:: > Yes. Style(9) says don't do this unless the code is really complicated:
:: >
:: > Parts of a for loop may be left empty. Do not put declarations inside
:: > blocks unless the routine is unusually complicated.
:: >
:: > I suspect that the stuff you are working on is complicated enough to
:: > justify their use. Style(9) doesn't say never do this.
::
:: I personally don't like it, but I think you are correct.
:
:I personally hate it too, and never use it. In most cases it argues
:for simpler code. However, sometimes the code isn't that easy to
:simplify and still have it be understandable and/or perform
:adequeately. It should be the exception rather than the rule.
:
:Warner
Remember, folks, we aren't robots here. Style(9) is useful as a guide
but that is about as far as it goes. There is a lot of silly junk in
there and it wastes everyones time to natter on about it, not to mention
committing little tweaks to other people's code. It's one thing for
us, as a community, to decide to get rid of __P() for example (which I
agree with), quite another to make annoying little commits changing minor
things to pieces of code that you are not otherwise working on.
Style(9) makes me think that somebody is trying to make programmers
write code like automatons rather then like people. For example, it
advocates utterly ridiculous, unreadable messes like this:
if (a) {
x;
y;
} else
/*
* HITHERE!
*/
b;
Which actually exists in the FFS code somewhere. Unbelievable.
This business about declaration placement is really in the eye of the
beholder. I'm sure the older people remember compilers breaking on
sub-block declarations, but I have not personally seen that sort of
breakage in many years. It is also easy to go too far the other way,
and I will readily admit that my 'mask' declaration in three side-by-side
sub-blocks was kind of silly. But, that said, it made far more sense to
move it up one level rather then move it up all the way to the top of
the procedure. I can't tell you how difficult it is to read and
understand a procedure that makes 30 declarations at the top, many of
which are used just once or twice somewhere deeply nested in the middle.
Yuch. It's as though code modularity ends at the procedure boundary
for a lot of people.
I'm not saying that people can't do cleanups of my code, but at the
very least if it is something I committed recently you should email me
a heads up and make sure the file isn't still under active development.
And I *DO* draw the line. I'll suffer splitting assignments off of
the declaration, the removal or addition of blank lines between
comments and code, and I'll even suffer the absolutely ridiculous blank
line placed after a procedure's open brace when a procedure has no
declarations. BUT, I draw the line at moving variable declarations
around or removing braces that were emplaced to make the code more
readable.
-Matt
Matthew Dillon
<dillon@backplane.com>
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200204020428.g324SkE14265>