From owner-cvs-all Mon Apr 1 20:29:13 2002 Delivered-To: cvs-all@freebsd.org Received: from apollo.backplane.com (apollo.backplane.com [216.240.41.2]) by hub.freebsd.org (Postfix) with ESMTP id 103FF37B416; Mon, 1 Apr 2002 20:28:52 -0800 (PST) Received: (from dillon@localhost) by apollo.backplane.com (8.11.6/8.9.1) id g324SkE14265; Mon, 1 Apr 2002 20:28:46 -0800 (PST) (envelope-from dillon) Date: Mon, 1 Apr 2002 20:28:46 -0800 (PST) From: Matthew Dillon Message-Id: <200204020428.g324SkE14265@apollo.backplane.com> To: "M. Warner Losh" 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 ... References: <200204012351.g31NpO890339@freefall.freebsd.org> <20020401.175136.106024419.imp@village.org> <20020401201130.K207@locore.ca> <20020401.181628.15900667.imp@village.org> Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG :In message: <20020401201130.K207@locore.ca> : Jake Burkholder 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 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 To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message