C99: Suggestions for style(9)

[M. Warner Losh]

In message: <20090428114754.GB89235 at server.vk2pj.dyndns.org>
            Peter Jeremy <peterjeremy at optushome.com.au> writes:
: >Maybe using all of these changes is a bit too big change at once, but 
: >I'd like some of them applied to style(9). So, please consider the 
: >proposed changes individually and not a as a all-or-nothing package.
: 
: One area you do not address is code consistency.  Currently, the
: FreeBSD kernel (and, to a lesser extent, userland) are mostly style(9)
: compliant - ie the entire codebase is mostly written in the same
: style.  Whilst you may not like it (and I don't know that anyone
: completely agrees with everything in style(9)), it does mean that
: the code is consistent.
: 
: Changing style(9) in a way that is not consistent with current code
: means that either existing code must be brought into line with the
: new standard - which incurs a one-off cost - or the code base becomes
: split into "old" and "new" style - incurring an ongoing maintenance
: cost as maintainers switch between styles.  Both approaches incur a
: risk of introducing new bugs.
: 
: Note that I'm not saying that style(9) can never be changed, I'm just
: saying that any change _will_ incur a cost and the cost as well as
: the potential benefits need to be considered.

This is my biggest worry about changing style(9) quickly.  We don't
want needless churn in the tree for just style changes since it makes
it harder to track bug fixes into the past.

: [Reduce declaration scope as far as possible, initialise variables where
:  they are declared, sort by name]
: 
: Whilst my personal preference is for the style suggested by Christoph
: (and I generally agree with the points he makes), this is also the
: change that incurs the biggest stylistic change.  This is not a change
: that can be practically retrofitted to existing code and therefore its
: implementation would result in a mixture of code styles - increasing
: the risk of bugs due to confusion as to which style was being used.  I
: am not yet sure whether the benefits outweigh the costs,

This is the biggest one, and I think it may be too soon.  Also, we
need to be careful on the initialization side of things because we
currently have a lot of code that looks like:


	struct foo *fp;
	struct bar *bp;

	fp = get_foo();
	if (!fp) return;
	bp = fp->bp;

this can't easily be translated to the more natural:

	struct foo *fp = get_foo();
	struct bar *bp = fp->bp;

since really you'd want to write:

	struct foo *fp = get_foo();
	if (!fp) return;
	struct bar *bp = fp->bp;

which isn't legal in 'C'.  However, we have enough where this isn't
the case that it should be allowed.  We should separate the
initialization part of this from the scope part of this too.  The
initialization part is already leaking into the code, while instances
of the scope restriction is rarer.

: [Don't parenthesize return values]
: >Removed, because it does not improve maintainability in any way.
: 
: This change could be made and tested mechanically.  But there is
: no justification for making it - stating that the existing rule
: is no better than the proposed rule is no reason to change.

I'm fine with this.

: [No K&R declarations]
: >Removed, because there is no need to use them anymore.
: 
: Whilst this is a change that could be performed mechanically, there
: are some gotchas relating to type promotion (as you point out).
: The kernel already contains a mixture of ANSI & K&R declarations and
: style(9) recommends using ANSI.  IMHO, there is no need to make this
: change until all K&R code is removed from FreeBSD.

I'm fine with this change.

: [ Don't insert an empty line if the function has no local variables.]
: 
: This change could be made and tested mechanically.  IMHO, this change
: has neglible risk and could be safely implemented.

I'm fine with this change.

: >> +.Sh LOCAL VARIABLES
: 
: >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.
: 
: Every version of gcc that FreeBSD has ever used would do this for
: primitive types when optimisation was enabled.  This approach can
: become expensive in stack (and this is an issue for kernel code) when
: using non-primitive types or when optimisation is not enabled (though
: I'm not sure if this is supported).  Assuming that gcc (and icc and
: clang) behaves as stated in all supported optimisation modes, this
: change would appear to be quite safe to make.

Agreed, in general.  We don't want to optimize our code style based on
what one compiler does, perhaps on x86.

Warner