Re: [dev] [sbase] style

From: Dimitris Papastamos <sin_AT_2f30.org>
Date: Tue, 18 Nov 2014 10:52:59 +0000

> no block for single statement (optional block? always block? discuss)

It depends. The following are valid.

if (v) {
        bla;
} else {
        bla;
        bla;
}

if (v) {
        bla;
        bla;
} else {
        bla;
}

if (v)
        bla;
else
        bla;

The following are invalid.

if (v) {
        bla;
        bla;
} else
        bla;

if (v)
        bla;
else {
        bla;
        bla;
}

Of course use of the ternary operator can be applied wherever it
makes sense.

> (what if inner statement uses block, braces outside? what if one of
> two branches uses block?)
>
> branches
> --------
> if (cond)
> foo();
> else {
> bar();
> baz();
> }
>
> or
>
> if (cond) {
> foo();
> } else {
> bar();
> baz();
> }

The latter.

> inner statement block
> ---------------------
> if (p)
> while (cond) {
> foo();
> bar();
> }
>
> or
>
> if (p) {
> while (cond) {
> foo();
> bar();
> }
> }

The latter.

> headers
> =======
> system headers (#include <...>) in alphabetical order
> empty line
> local headers (#include "...") in alphabetical order
> if there is any reason to change order, comment to explain

system headers first, then a newline, then libc headers, then a
newline then local headers.

libc headers are sorted lexicographically, system and local headers are
included in the proper order. No preprocessor guards for local headers.

> variadic macros
> ===============
> yay or nay?

yes.

> file layout
> ===========
> check dwm.c for example
> leading comment with LICENSE notice and short explanation of file
> headers
> macros
> types
> function declarations
> global variables
> function definitions
> main

I propose:

LICENSE header
headers
macros
types
function declarations
global vars
usage /* this could also go below for consistency but I like it here */
main
function definitions

> functions
> =========
> declarations at top of file
> declarations, definitions, in alphabetical order (except main, at end of file)
> static if not used outside current file (what's the right term?
> compilation unit?)
> definitions have modifiers and return type on own line
> { on own line (function definitions are special case of block as they
> cannot be nested)

Also parameters should not be named in function declarations.

> C version
> =========
> use C99 (why not C11? I really like anonymous unions/structs)

C99.

> do not use for loop initial declarations (why?)

Declare variables at the function level.

> types
> =====
> user defined types start with a capital letter
> when possible typedef struct {} Name;

Debatable. I'd like to hear more opinions on this.

> line length
> ===========
> we aren't limited to 80 columns in our terminals anymore, no need to
> pretend we are
> if using more columns makes code more readable, do so
> it also means we can fit more code on a single screen, less scrolling
> (some hard limit so we don't have 300 character lines?)

No. 80 columns per line is sane. This is not a hard rule and you can
easily have printf() or similar spanning that limit.
The idea is that heavily indented code is probably broken in the first
place and that all good, sane and elegant code should really fit in 80
columns per line. This implies sane naming conventions and proper use
of continue and break.

> tests (boolean)
> ===============
> do not test against 0 explicitly (e.g. if (!strcmp(p, q)) instead of
> if (strcmp(p, q) == 0)) (discuss)

It should be the latter. See my other post here for details.

> do use compound assignment and test (e.g. if (!(p =

Debatable. I prefer this to be on separate lines.

> malloc(sizeof(Type)))) ...; )

p = malloc(sizeof(*p));

> early returns/exits
> ===================
> if needed use label + goto for cleanup instead of multiple nested
> levels, however this can normally be avoided (discuss)

No it cannot be avoided easily.

> return/exit/fail early instead of using multiple levels of tests

I am inclined to use exit() everywhere, even in main(). Do not
use EXIT_SUCCESS or EXIT_FAILURE.

> do not clean up on fatal errors (just enprintf())

Other than reporting the correct exit status as specified in POSIX
it should be ok. Please note that if the output device is full we
need to correctly exit with a failure status if the program has still
output data to be flushed. This is only a concern in cases where the
program will terminate successfully but failing to print its output.

For large tools, we probably do want to clean up. I can elaborate
on this further if you are interested but we did have a discussion on
IRC about this.

> remove linebreaks
> -----------------
> switch (a) {
> case '1': foo(b); break;
> case '2': bar(c); break;
> case '3': baz(d); break;
> default :
> enprintf(2, "RTFM\n");
> }

Agreed. This is to be done on a case by case basis. We do not need
to worry about consistency if it makes a particular piece of code
more readable. Sometimes elegance comes as a consequence of not
applying consistency principles blindly.

> more alignment
> --------------
> somefunc(foo .bar, baz );
> somefunc(bonzai.bar, qux );
> somefunc(blah .bar, blargh);

I do not like this.

Everything else I did not comment on I agree as is. Can you please
post a patch that adds a new file called STYLE to sbase? We can commit
that and then fix it incrementally.

Thanks for your work,
Dimitris
Received on Tue Nov 18 2014 - 11:52:59 CET

This archive was generated by hypermail 2.3.0 : Tue Nov 18 2014 - 12:00:11 CET