C99: Suggestions for style(9)

[Christoph Mallon]

Peter Jeremy schrieb:
> On 2009-Apr-26 09:02:36 +0200, Christoph Mallon <christoph.mallon at gmx.de> wrote:
>> as some of you may have noticed, several years ago a new millenium 
>> started and a decade ago there was a new C standard.
> 
> Your implication that FreeBSD is therefore a decade behind the times
> is unfair.  Whilst the C99 standard was published a decade ago,
> compilers implementing that standard are still not ubiquitous.
> 
>> HEAD recently 
>> switched to C99 as default (actually gnu99, but that's rather close).
> 
> Note that gcc 4.2 (the FreeBSD base compiler) states that it is not
> C99 compliant.

It's good enough. Only one proposed change (mixing declarations and
statements) actually needs C99. GCC supports this for many years.

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

I see no problem with removing/changing the existing rules to make sure
new code is not forced to use them just because it always has been done
this way. Nobody will change all the source code at once, because its
insane to check and change millions of lines of code at once.
Considering this, you pretty much said, that style(9) cannot be changed.
I see no problem in improving the rules and then gradually changing the
code when it is convenient. Especially new code will profit from the
changes, because it is not condemned to use the old rules for eternity.
There are several C99 features used already, e.g. designated initializers:
	bla bli = { .blub = "foo", .arr[0] = 42 };
Do you suggest that this should not be used, because it is inconsistent 
with all the other existing compound initialisations?

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

There is no need to change all the existing code at once. But new code
should be enabled to use the improved style.

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

Just remove the rule. There's no need to add more redundant parentheses.
Again: There is no need to change all existing code at once, but the
rule is removed so that not anymore redundant parentheses are added.

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

Again: No need to change all the code at once, but the rule is removed,
so not any more old style declarations are added.
The current attempts to compile FreeBSD using LLVM/clang revealed
several existing bugs in functions with old style declarations.

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

Again: No need for immediate global change, but just do not add anymore
of those. There are already quite some functions, which do not have the
unnecessary empty line.

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

Most local variables are scalars (pointers, ints, ...). A struct or 
array as local variable is rare and then usually used in one context. So 
IMO this is fine. Even considereing the extremly rare case of multiple 
non-scalar declarations in a function, then a compiler can coalesce them 
if their life ranges are disjoint. It is easier to do so for a compiler 
to do so, if they are declared with smaller life ranges, example:

struct Foo { int  x[16]; };
struct Bar { int* y[16]; };

void f(struct Foo*);
void g(struct Bar*);

void e(int x)
{
	struct Foo a;
	struct Bar b;
	if (x) {
		f(&a);
	} else {
		g(&b);
	}
}

Stack usage:
	subl	$140, %esp

moving the declarations into the branches:
	subl	$76, %esp

So, apart from improved readability, it also can lead to better code. 
But I consider the latter way less important the benefits observed by a 
reader of the code.

	Christoph