Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 2 May 2009 08:36:58 +0100
From:      David Malone <dwmalone@maths.tcd.ie>
To:        Christoph Mallon <christoph.mallon@gmx.de>
Cc:        FreeBSD Hackers <freebsd-hackers@freebsd.org>, Roman Divacky <rdivacky@freebsd.org>, Ed Schouten <ed@freebsd.org>, Warner Losh <imp@freebsd.org>, Maxim Sobolev <sobomax@freebsd.org>
Subject:   Re: C99: Suggestions for style(9)
Message-ID:  <20090502073658.GA65633@walton.maths.tcd.ie>
In-Reply-To: <49F4070C.2000108@gmx.de>
References:  <49F4070C.2000108@gmx.de>

next in thread | previous in thread | raw e-mail | index | archive | help
FWIW, I'd rarely support changing style(9), unless it is actually
causing people to write bad code. It's designed to produce consistent
code, and changing it does not encourage consistency.

> >-Do not put declarations
> >-inside blocks unless the routine is unusually complicated.
> >+Prefer declaring loop iterators in the for-statement to limit their scope.

I usually don't like this style - I like being able to review the
variables used in a function at the top.

> >-When declaring variables in functions declare them sorted by size,
> >-then in alphabetical order; multiple ones per line are okay.
> >+When declaring variables in functions declare them sorted in alphabetical 
> >order;
> >+prefer one declaration per line, especially when pointer declarators or 
> >+initializations are involved.

The change to prefer one declaration per line seems like an unnecessary,
except to better accomodate the declare-at-initialisation below.

> >+Prefer initializing variables right at their declaration.

While I buy your argument about scope, as I said, I prefer to see
everything declared at the top. I don't think it outweighs the
advantage of being able to look through the declarations.

> >+When the value is not supposed to change in the function, make the 
> >variable
> >+const.

Using const seems sensible, and a good example of a time where
declaring at initialisation makes sense.

> >+This makes it easier for a reader to identify the where the value of a 
> >variable
> >+originates.

I'm not sure I buy this - the initialisation is unlikely to move in
a piece of code, so it's as hard to find now as it was before. Editors
supporting finding declarations should be able to find initialisations
just as easily. (I'm old fashioned and do it via regexps.)

Note that I also compile code from time to time to versions of
FreeBSD that don't support C99. I understand that this will get
more difficult over time, but mixing declarations and code is one
of the things that I trip over that are just an annoying unnecessary
change. For me it makes my life more difficult for a small gain.

> >+Do not reuse the same variable in a different context, delare a new 
> >variable.

I buy this largely - though it should be applied with common sense
as usual.

> >-Values in
> >-.Ic return
> >-statements should be enclosed in parentheses.
> >-.Pp

This is another change that isn't worth making - style(9) has had
this rule for years and changing it because you don't think it adds
anything isn't a good reason.

> >-Use ANSI function declarations unless you explicitly need K&R 
> >compatibility.
> >+Use ANSI function declarations.

Seems reasonable for modern code - I guess it might be worth noting
that when converting from K&R to ANSI changes should be made with
care to avoid the sort problems you mention.

> (It's a bug in GCC that it does not complain about the former.)

I thought gcc had some magic glue to make functions with ANSI
declarations and K&R definitions work. Bruce would know the details.

> Empty line when there are no local variables:
> Removed, because it does not improve readability and it actually hinders 
> readability for very short functions, e.g. static inline functions, 
> which just consist of one or two lines of code without any local 
> variables except for parameters. Further, it makes even less sense with 
> the changed recommendations for declarations.

FWIW, I buy the empty line rule as a matter of consistency. A better
reason for getting rid of it (IMHO) would be that it is one of the
rules that is not so well followed by our existing code base. On
balance I'd leave this rule here, because I don't think the changes
to decalarations are good and because changing for no good reason
produces less consistent code in the long run.

(The example of very short static inline functions is probably a
good one where no one would complain anyway if the code was missing
the blank line. As always, style needs to be applied with good sense.)

> Last, but definitely not least, I added this paragraph about the use of 
> local variables. This is to clarify, how today's compilers handle 
> unaliased local variables. There is absolutely no need to reuse them in 
> different contexts. Trying to optimise stack usage in this way is 
> misguided and is a source of bugs, when a reused variable is not as dead 
> as thought. For more reasons, please read the quoted diff.
> Maxim, you requested this paragraph: Does this addition suit you?

This paragraph looks useful. If I were you I'd hook it in to the
recommendation to not reuse the same variable in a different context.
The only thing is that it reads a bit more like programming advice
rather than style advice.

I'd also suggest you run any changes by Bruce.

	David.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090502073658.GA65633>