C99: Suggestions for style(9)

[Christoph Mallon]

M. Warner Losh schrieb:
> 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.

I'm not saying that all the code has to be changed at once, but no new 
code of the "old" style should be added. E.g. you should never use K&R 
declarations when adding a new function. And if you are going to make a 
big change somewhere, then it is sensible to use the "new" style.

> : [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

This is the reason, why I explicitly mentioned, that the proposed 
changes should be examined independently.

> 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

You're mistaken, this is perfectly legal C. See ISO/IEC 9899:1999 (E)
§6.8.2:1. In short: you can mix statements and declarations.

> 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.

Sorry, I do not understand these sentences.

> : [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.

Would you commit these three proposals?

> : >> +.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.

I'm not sure whether I understand this - in particular the last three words.

	Christoph