WARNING: This is the _old_ Lustre wiki, and it is in the process of being retired. The information found here is all likely to be out of date. Please search the new wiki for more up to date information.

Coding Guidelines

From Obsolete Lustre Wiki
Revision as of 06:38, 28 August 2007 by Scjody (talk | contribs) (Add autoconf info)
Jump to navigationJump to search

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.

Guidelines

1. 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-CFS project).

2. Blocks should be indented by 8 spaces.

3. 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.

  /* -*- mode: c; c-basic-offset: 8; indent-tabs-mode: nil; -*-
   * vim:expandtab:shiftwidth=8:tabstop=8:
   */

4. 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.

5. Don't 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:

/[ \t]$/

6. Don't use "inline" unless you're doing something so performance critical that the function call overhead will make a difference -- in other words: 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.

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

Great detail

a. When you wrap, the next line should start after the parenthesis:

  right:
 
  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) {
  }
 

b. If you're wrapping put the operators at the end of the line, and if there are no parentheses indent 8 more:

  off = le32_to_cpu(fsd->fsd_client_start) +
          cl_idx * le16_to_cpu(fsd->fsd_client_size);
 

c. Binary and ternary (but not unary) operators should be separated from their arguments by one space.

  a++;
  b |= c;
  d = f > g ? 0 : 1;
 

d. Function calls should be nestled against the parentheses, the parentheses should crowd the arguments, and one space after commas:

  right: do_foo(bar, baz);
  wrong: do_foo ( bar,baz );

e. All if, for, while, etc. expressions should be separated by a space from the parenthesis, one space after the semicolons:

  for (a = 0; a < b; a++)
  if (a < b || a == c)
  while (1)
 

f. 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".

  int foo(void)
  {
          if (bar) {
                  this();
                  that();
          } else if (baz) {
                  ;
          } else {
                  ;
          }
          do {
                  cow();
          } while (0);
  }

g. If one part of a compound if block has braces, all should.

  right:
 
  if (foo) {
          bar();
          baz();
  } else {
          salmon();
  }
 
  wrong:
 
  if (foo) {
          bar();
          baz();
  } else
          moose();

h. When you make a macro, protect those who might call it by using do/while and parentheses; line up your backslashes:

  right:
 
  #define DO_STUFF(a)                              \
  do {                                             \
          int b = (a) + MAGIC;                     \
          do_other_stuff(b);                       \
  } while (0)
 
  wrong:
 
  #define DO_STUFF(a) \
  { \
          int b = a + MAGIC; \
          do_other_stuff(b); \
  }

i. If you nest preprocessor commands, use spaces to visually delineate:

  #ifdef __KERNEL__
  # include <goose>
  # define MOOSE steak
  #else
  # include <mutton>
  # define MOOSE prancing
  #endif

j. For very long #ifdefs include the conditional with each #endif to make it readable:

  #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__ */ 

k. 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 *:

 
 /* 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. */

l. 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).

m. 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.

n. The types and printf/printk formats used by Lustre code are:

   __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)