Date: Sun, 17 May 2009 14:45:16 +0400 From: Stanislav Sedov <stas@FreeBSD.org> To: Christoph Mallon <christoph.mallon@gmx.de> Cc: FreeBSD Hackers <freebsd-hackers@freebsd.org> Subject: Re: C99: Suggestions for style(9) Message-ID: <20090517144516.331b01a8.stas@FreeBSD.org> In-Reply-To: <49FAE4EA.1010205@gmx.de> References: <49F4070C.2000108@gmx.de> <20090428114754.GB89235@server.vk2pj.dyndns.org> <49FAE4EA.1010205@gmx.de>
next in thread | previous in thread | raw e-mail | index | archive | help
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On Fri, 01 May 2009 14:02:50 +0200
Christoph Mallon <christoph.mallon@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.
> > [ 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.
>
What seems to you as unnecessary rule may be of a great use for other
code users. For me it improves the code readability as an empty line
at the start clearly points that there're no local variables used. I
don't see enough argumentation for removing this rule.
> >>> +.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. Aliasing behavior is stritcly described in
ISO C99 standard, so there's a good point to enforcing strict-aliasing clear
code in our kernel. On the other hand the big work should be done on clearing
the existing code before any rule on this can be enforced.
- --
Stanislav Sedov
ST4096-RIPE
-----BEGIN PGP SIGNATURE-----
iEYEARECAAYFAkoP6rwACgkQK/VZk+smlYHdAACeJo64Mc0syCLtXq93yg0f87Y7
T2kAn1gLof6OMcHHs3EbRYTx7QE5NwU8
=5waq
-----END PGP SIGNATURE-----
!DSPAM:4a0fea9d994295559415935!
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20090517144516.331b01a8.stas>