Coding Guidelines

Note: ''All Lustre developers should follow the guidelines in this page very strictly to avoid problems during code merges later on. Please make the required changes to the default formatting rules in the editor you use to comply to the guidelines below.

Writing "Beautiful" Code
More important than the physical layout of code (see Formatting Guidelines) is clarity and readability. These guidelines will help you write clear, readable code:


 * Separate "ideas" in the program layout. For example, use blank lines to group related statements and separate unrelated statements.


 * Make declarations easy to read. Declarations should always be separated from the functional code.  Declarations should be made as locally as possible to the code they are being used.  Placing one variable per line, with the names aligned vertically, makes checking types quicker and easier.


 * Break the 80-chars-per-line rule if needed for readability. For example, use a longer line if the 80-chars-per-line rule requires breaking indentation rules or doesn't allow procedure call parameters to be visually contained between the brackets or results in code spread over too many lines. Lines shouldn't be excessively long but today's screen resolutions no longer require line wraps at 80 chars.


 * Choose names with care. Well-chosen names are essential for readability and clarity.
 * The name of a procedure or variable or structure member should be descriptive, without being misleading.
 * The name should should be selected so that it isn't easily confused with names already in use. For example, don't choose a name that could be turned into another valid name (to the compiler) with a simple spelling mistake, such as req_portal and rep_portal (taken from our own code!).
 * Use a longer name if required for clarity (while avoiding extremely long names).


 * Use assertions intelligently. When over-used, assertions can hurt performance, but when used properly, they combine the roles of active comment and software fuse.  As an active comment they tell you something about the program that you can trust more than a comment.  As a software fuse, they provide fault isolation between subsystems by letting you know when and where invariant assumptions are violated.


 * Don't be overly clever. It's only one step from clever to tricky - consider this example:

t = a; a = b; b = t; /* dumb swap */

a ^= b; b ^= a; a ^= b; /* clever swap */

In this case, the "cleverness" is confined to a tiny part of the code. However, if the clever code had a sub-structure, the sub-structure would be very hard to work on. It would be difficult to know the effects of a change without understanding completely the environment in which you're working.

''Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it.'' - Brian W. Kernighan

"Beautiful" code supports code quality because it improves communication between the code author and the code reader. Code readers are also code modifiers, so the quality of communication can lead either to a virtuous circle of improving quality or a vicious cycle of degrading quality.

Formatting Guidelines

 * There should be no tabs in any Lustre or LNET files. The exceptions are libsysio (maintained by someone else), ldiskfs and kernel patches (also part of a non-Lustre Group project).


 * Blocks should be indented by 8 spaces.


 * Variables should be declared one per line, even if there are multiple variables of the same type.

 /* -*- mode: c; c-basic-offset: 8; indent-tabs-mode: nil; -*- * vim:expandtab:shiftwidth=8:tabstop=8: */
 * New files should contain the following along with the license boilerplate. This will cause vim and emacs to use spaces instead of tabs for indenting.  If you use a different editor, it also needs to be set to use spaces for indening Lustre code.


 * All lines should wrap at 80 characters. If it's getting too hard to wrap there, you probably need to break it up into more functions. In some cases, it is acceptable to remove a few spaces between function arguments to avoid overflowing onto the next line.

 /[ \t]$/
 * Do not have spaces or tabs on blank lines or at the end of lines. Find these with some regexps in your patch (grep, or in vim) before attaching it to bugzilla:

Alternatively, if you use vim you can put this line in your vimrc file, which will highlight whitespace at the end of lines and spaces followed by tabs in indentation (only works for C/C++ files):  let c_space_errors=1

Or you can use this command, which will make tabs and whitespace at the end of lines visible for all files (but a bit more discretely):  set list listchars=tab:>\ ,trail:$

In emacs, you can use (whitespace-mode) or (whitespace-visual-mode) depending on version. You should also consider using (flyspell-prog-mode).


 * Don't use "inline" unless you're doing something so performance critical that the function call overhead will make a difference -- in other words: almost never. It makes debugging harder.

All of our wrapping, parenthesis, brace placement, etc. rules are basically Linux kernel rules, which are basically K&R. For those of you in need of a refresher, great detail is provided below.


 * For Autoconf macros, follow the style suggested in the autoconf manual.

 AC_CACHE_CHECK([for EMX OS/2 environment], [ac_cv_emxos2],    [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([], [return __EMX__;])], [ac_cv_emxos2=yes], [ac_cv_emxos2=no])]) or even  AC_CACHE_CHECK([for EMX OS/2 environment],                   [ac_cv_emxos2],                    [AC_COMPILE_IFELSE([AC_LANG_PROGRAM([],                                                        [return __EMX__;])], [ac_cv_emxos2=yes], [ac_cv_emxos2=no])])

Great detail
 int          len; int          count; struct inode *inode;
 * Each variable declaration should be on a separate line, with the names aligned to the same column:

 if (foo) bar;
 * Even for short conditionals, the operation should be on a separate line:

 right:
 * When you wrap, the next line should start after the parenthesis:

variable = do_something_complicated(long_argument, longer_argument,                                     longest_argument(sub_argument, foo_argument),                                     last_argument); if (some_long_condition(arg1, arg2, arg3) < some_long_value &&     another_long_condition(very_long_argument_name, another_long_argument_name) >     second_long_value) { }  wrong:

variable = do_something_complicated(long_argument, longer_argument,                   longest_argument(sub_argument, foo_argument),                    last_argument); if (some_long_condition(arg1, arg2, arg3) < some_long_value &&             another_long_condition(very_long_argument_name, another_long_argument_name) >                   second_long_value) { }  off = le32_to_cpu(fsd->fsd_client_start) + cl_idx * le16_to_cpu(fsd->fsd_client_size);  a++; b |= c; d = f > g ? 0 : 1;  right: do_foo(bar, baz); wrong: do_foo ( bar,baz );  for (a = 0; a < b; a++) if (a < b || a == c) while (1)  int foo(void) {         if (bar) { this; that; } else if (baz) { ;         } else { ;         }          do { cow; } while (0); }  right:
 * If you're wrapping put the operators at the end of the line, and if there are no parentheses indent 8 more:
 * Binary and ternary (but not unary) operators should be separated from their arguments by one space.
 * Function calls should be nestled against the parentheses, the parentheses should crowd the arguments, and one space after commas:
 * All if, for, while, etc. expressions should be separated by a space from the parenthesis, one space after the semicolons:
 * Opening braces should be on the same line as the line that introduces the block, except for function calls. Closing braces get their own line, except for "else".
 * If one part of a compound if block has braces, all should.

if (foo) { bar; baz; } else { salmon; } <pre style="background:lightgrey;"> wrong: if (foo) { bar; baz; } else moose; <pre style="background:lightgrey;"> right: #define DO_STUFF(a)                             \ do {                                            \ int b = (a) + MAGIC;                    \ do_other_stuff(b);                      \ } while (0) <pre style="background:lightgrey;"> wrong: #define DO_STUFF(a) \ { \         int b = a + MAGIC; \ do_other_stuff(b); \ } <pre style="background:lightgrey;"> #ifdef __KERNEL__ # include # define MOOSE steak #else # include # define MOOSE prancing #endif <pre style="background:lightgrey;"> #ifdef __KERNEL__ # if LINUX_VERSION_CODE >= KERNEL_VERSION(2,5,0) /* lots of    stuff */ # endif /* KERNEL_VERSION(2,5,0) */ #else /* !__KERNEL__ */ # if HAVE_FEATURE /* more * stuff */ # endif #endif /* __KERNEL__ */ <pre style="background:lightgrey;"> /* This is a short comment */ /* This is a multi-line comment. I wish the line would wrap already, * as I don't have much to write about. */ <pre style="background:lightgrey;"> right:
 * When you make a macro, protect those who might call it by using do/while and parentheses; line up your backslashes:
 * If you nest preprocessor commands, use spaces to visually delineate:
 * For very long #ifdefs include the conditional with each #endif to make it readable:
 * Comments should have the leading /* on the same line as the comment, and the trailing */ at the end of the last comment line. Intermediate lines should start with a * aligned with the first line's *:
 * Conditional expressions read more clearly if boolean expressions are implicit, but non-boolean and pointer expressions compare explicitly with 0 and NULL respectively - e.g...

if (inode != NULL &&    /* valid inode? */       !writing &&          /* not writing? */       ref_count == 0)      /* no more references? */          do_this; <pre style="background:lightgrey;"> wrong:

if (inode &&            /* valid inode? */       writing == 0 &&      /* not writing? */       !ref_count)          /* no more references? */          do_this; <pre style="background:lightgrey;"> right:
 * Parentheses and line breaks help make conditional expressions more readable.

if (a->a_field == 3 ||      ((b->b_field & BITMASK1) && (c->c_field & BITMASK2))) do this; <pre style="background:lightgrey;"> wrong:

if (a->a_field == 3 || b->b_field & BITMASK1 && c->c_field & BITMASK2) do this <pre style="background:lightgrey;"> __u64                LPU64/LPX64/LPD64 (unsigned, hex, signed) size_t               LPSZ (or cast to int and use %u / %d) __u32/int            %u/%x/%d (unsigned, hex, signed) (unsigned) long long %llu/%llx/%lld loff_t               %lld after a cast to long long (unfortunately)
 * Function declarations absolutely should NOT go into .c files, unless they are forward declarations for static functions that can't otherwise be moved before the caller. Instead, the declaration should go into the most "local" header available (preferrably *_internal.h for a given piece of code).
 * Structure and constant declarations should not be declared in multiple places. Put the struct into the most "local" header possible.  If it is something that is passed over the wire it needs to go into lustre_idl.h, and needs to be correctly swabbed when the RPC message is unpacked.
 * The types and printf/printk formats used by Lustre code are:

''