On Mon, 2008-09-08 at 08:27 +0200, Henning Rogge wrote:
> My suggestions for coding style are simple:
> 
> No (or nearly no) macros except for constants. Most macros make the code 
> unreadable and difficult to debug.
Instead use inline functions.

> No code in header files, it does belong into the source files.
... unless it is in inline functions.
And contrary to common belief: It is a myth (and/or cargo cult), that
simply inlining C code makes it faster.
Yes, there are situations where this is true (like one line accessors).
But as soon as you have one if()[1] or while() (or - God beware - a
system call), the cost of an additional function call is very probably
neglectable compared to the rest of the function. And the #define/inline
normally blows up the generated code bringing more pressure on the L1/L2
caches and the memory accesses (and this is even more the case the
larger/faster the CPU is).
That's actually the reason for the -finline-limit parameter: To catch
cases where an inline function suddenly explodes (for whatever reason).

> No "source code compression". There is no reason the write extra tricky source 

ACK. E.g. for "if((ptr = malloc(22)) != NULL)". Make it two lines and no
one has to count the parentheses. Let alone on more complex examples.

> code only because it saves a few lines if the longer (and more readable) code 
> would result in a similar binary.
Yes but "readability" is rather subjective. Personally I find the "0 ==
xx" completely unreadable (And yes, I know why people do that. And that
is with gcc with it's warnings since years not really necessary
anymore).

> No scripts to create code. Generated code is a nightmare to read and to debug.
*eg* Only if your generator is crap;-) Let him generate readable code.
And no one really debugs generated code. You debug and fix the
generator[2].
At most one reads the generated code to improve the generator.

If you have a generator, it must check the input completely (and reject
it if it has bugs[4]).

And generating code something implies that you have input data which
will change overtime quite frequently (and by people which are not
expected to fix the source) and it is less work and risk to have that
additional tool. There is no need to generate code for a bunch of never
changing data.

BTW: If your generator generates lots of code, you very probably doing
something wrong anyways. In my experience, the generators evolve into
producing tables which drive normal functions.

As for an example in olsrd: The builddata.c merely exists to avoid more
than one definition. The simply workaround is to add 3 -D parameters
containing the values to CPPFLAGS and be done. But then I don't know how
many useless copies of them end up in the final binaries.
Granted, in the above cases, the wasted space is quiet small. But one
could generate much longer -D parameters containing full-blown
functions .....

> Clear namespaces for all functions/global variables in a .c file. Maybe a 
> common prefix which is a "compressed" but easy to recognize version of the 
> filename itself ( I use the prefix "pbb_" for all packet_bb parser 
> functions).

If someone is interested in mine (additional to the above;-):

The most important one: Do not copy "logic" (read: almost always code).
Abstract it. Or will have a maintenance nightmare sooner or later.

Declare file local functions and data "static". Otherwise give a
declaration in it's header file.

Use "const" if a variable shouldn't/isn't changed.
Rationale: One can easily see it and the compiler checks it too.

Define variables in the most inner block.
Rationale: No one wants to scroll up and look for the definition.

Use meaningful function and variable names.
Rationale: It makes understanding the code easier and avoids explicit
comments.

Remove all useless casts, e.g. casting the return value of malloc() and
friends since void * are automatically convertible anyways.
Rationale: A type cast completely removes type checks of the compiler.
And we won't educate people to use casts[0] but the other way around.
Exception: Casts can be used to type-safety. Use it then.

Use "typedef" only for function types.
Rationale: All other uses obfuscate the source - especially true for
struct's and union's (as one forgets that you have a struct or union
there. And you won't look up .h fiels all the time).
Exceptions: You want some special kind of e.g. "int" to be used with a
special set of functions and use a typedef to make the type check
trigger wrong uses. "atomic_t" is the prime example in the Linux kernel
code.

Activate all remotely useful of warnings - especially -Werror. And yes,
e.g. -Wshadow (and others) let me find bugs and quirks and superfluous
stuff in other apps.

Make all .c and .h files self-confident.
Rationale: You do not want to look into foo.h after you include it in
bar.c and add 3 more #include lines just because foo.h needs them.

Minimize the number #include's (without violating the previous).
Rationale: Apart from the neglectable performance win, one can see at
one glance what stuff from which other .c files is used.

I see a .c file as the lowest level of modularization. So have 1 .h file
for every .c file containing the relevant extern's and #defines.
And #include that in it's .c file as the very first #include.
Rationale:
- Think of "OO design" (and yes, an object-oriented programming
  language is neither necessary nor sufficient for OO programming. It
  makes it easier and helps but that's all).
- That guarantees the above "self-confidence".
- So the compiler actually checks if the function declaration in the .h
  file actually matches the function definition (and yes, even olsrd had
  such differences which crept into it over time).

Always use the { ... } for if()/else, while(), for(), do { }
while(), ... No oneliner if(),while(), etc. - have the body on separate
lines.
Rationale: You have then one pattern of these (and not two) and reduce
the risk of conflicts on merging.

ad formatting: Use Linux kernel coding style[3] except
- the above "use { ... } everywhere"
- indent 4 (and not 8) positions.
- indent with spaces (and not tabs)
  Rationale: It is far easier to get it right. And there is only one
  true width of one tab - and that is 8.
a draft list of parameters for indent is
---- snip  ----
--k-and-r-style --indent-level4 --tab-size8
--swallow-optional-blank-lines --line-length132
--space-special-semicolon --no-space-after-casts --braces-on-if-line
--braces-on-struct-decl-line --no-tabs --cuddle-else --cuddle-do-while
---- snip  ----

	Bernd

[0]: IMHO *every* single type cast should have an comment explaining why
     it is needed. And *root cause* has to be explained, not a symptom
     like "gcc throws a warning and that got rid of it".
     That holds especially for beginner courses in C.
[1]: Which can't be optimized away by the compiler, etc. ....
[2]: Imagine you write a generator in C++ (which runs as part of the
     toolchain) which generates C code (which runs on the target) which 
     generates JavaScript code (which runs in the browser) and the
     HTML around.
     C++ and C use " as string delimiter. JavaScript can use " and ' but
     it should work together with the surrounding HTML. Have fun with
     correct quoting ....
     And it was the somewhat simplest solution to the problem.
[3]: GNU coding style is ugly and wastes lots of space.c. Even on a
     large screen and a large editor window nowadays, let alone on
     80x25.
[4]: Read [2] above. You really don't want an syntax error in the input
     file show up at run-time in the browser and be debugged, found and
     backtraced back through the toolchain as the toolchain just copies
     the error around.
-- 
Firmix Software GmbH                   http://www.firmix.at/
mobil: +43 664 4416156                 fax: +43 1 7890849-55
          Embedded Linux Development and Services





