C99: Suggestions for style(9)

[Christoph Mallon]

Stanislav Sedov schrieb:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On Fri, 01 May 2009 14:02:50 +0200
> Christoph Mallon <christoph.mallon at gmx.de> mentioned:
> 
>>> [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.
> 
> If only the rule gets removed this will lead to inconsistent code. Currently
> it is much easier to experienced leader to notice return statements with
> parenthesis around the value than without. Recall that people's eyes are
> build in way that they recognized entire expressions and not letter
> combinations.
I don't buy this for a simple reason: Parentheses are in many statements 
(if, while, for). The only thing which distinguishs a return statement 
from others is the word "return".

>>>>> +.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.
>>
> 
> I particulary like this change.
Why?

> Aliasing behavior is stritcly described in
> ISO C99 standard, so there's a good point to enforcing strict-aliasing clear
> code in our kernel.
If you like this addition because of this reason, I have to disappoint 
you: This addition has absolutly *nothing* to do with strict-aliasing.

> On the other hand the big work should be done on clearing
> the existing code before any rule on this can be enforced.
This addition is about improving readability for humans, because it 
simplifies the def-use-chains, and as a side effect sometimes leads to 
better generated code. It is not sensible to check millions of lines of 
code whether a variables are used in different contexts within a 
function before this can added.

Anyway, this is moot, because this thread was dead because every 
suggested improvement was rejected - especially this last improvement 
was rejected by the guy who requested it in the first place.

	Christoph