Page tree

Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

Indentation

Tabs should be used in all C files. 

Tabs are preferred to align variable names in structure and enum declarations. This may temporarily misalign them with other variable names in the structure that are using spaces for indentation, or consider fixing up the whole struct in the same patch if there aren't too many of them. In some cases (long variable names, long comments, nested unions) then it isn't practical to use full tabs to align the fields and spaces can be used for partial indentation. Moreover, the returned type and the function name should be on two different lines.

right:
void
func_helper(...)
{

...

Do not include spaces or tabs on blank lines or at the end of lines. Please ensure you remove all instances of these in any patches you submit to Gerrit. You can find them with grep or in vim using the following regexps /\[ \t\]$/ or use git git diff --check. 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 the version. You could also consider using (flyspell-prog-mode).

Functions & Variables

Functions that are more than a few lines long should be declared with a leading comment block that describes what the function does, any assumptions for the caller (locks already held, locks that should not be held, etc), and return values. DAOS uses the DOxygen markup style for formatting the code comments, as shown in the example here. \a argument can be used in the descriptive text to name a function argument. \param argument definition should be used to define each of the function parameters, and can be followed by [in][out], or [in,out] to indicate whether each parameter is used for input, output, or both. \retval should be used to define the function return values.

...

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

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


External function declarations absolutely should *NEVER* go into .c files. The only exception is forward declarations for static functions in the same file that can't otherwise be moved before the caller. Instead, the declaration should go into the most "local" header available (e.g. subsystem_internal.h for a given subdirectory). Having external function declarations in .c files can cause very difficult to diagnose runtime bugs, because the compiler takes the local declaration as correct, can not compare it to the actual function declared in a different file, and does not have a declaration in a header to compare it against, but the linker does not check that the number and type of function arguments match.

Function declarations in header files should include the variable names for the parameters, so that they are self explanatory in the header without having to look at the code to see what the parameter is:

right:
    void ldlm_lock_addref_internal(struct ldlm_lock *lock, enum ldlm_lock_mode lock_mode, int refcount, int rc);

wrong:
    void ldlm_lock_addref_internal(struct ldlm_lock *, int, int, int)

Structure fields should have a common prefix derived from the structure name, so that they are easily found in the code and tag-jump works properly. This avoids problems with generic field names like page or count that have dozens or hundreds of potential matches in the code.

Structure and constant declarations should not be declared in multiple places. Put the struct into the most "local" header possible.

Structure initialization should be done by field names instead of using positional arguments:

struct foo {
        int     foo_val;
        int     foo_flag;
        int     foo_count;
        void   *foo_ptr;
        void   *foo_bar_ptr;
        void   *foo_baz_ptr;
};

right:
void func(void *data)
{
        struct foo fooz = { .foo_val = 1, .foo_flag = 0x20, .foo_ptr = data, .foo_baz_ptr = param };
}

wrong:
void func(void *data)
{
        /* not sure which field is being initialized, breaks (maybe silently) if structure adds a new field */
        struct foo fooz = { 1, 0x20, data, param };
}

When allocating/freeing a single struct, use D_ALLOC_PTR() for clarity:

right:
        D_ALLOC_PTR(mds_body);
        D_FREE_PTR(mds_body);

wrong:
        D_ALLOC(mds_body, sizeof(*mds_body));
        D_FREE(mds_body, sizeof(*mds_body));

Macros

When you define a macro, protect callers by placing parentheses round every parameter reference in the body.

...

#ifdef __KERNEL__
# if HAVE_SOME_KERNEL_API
/* lots
   of
   stuff */
# endif /* HAVE_SOME_KERNEL_API */
#else /* !__KERNEL__ */
# if HAVE_ANOTHER_FEATURE
/* more
 * stuff */
# endif /* HAVE_ANOTHER_FEATURE */
#endif /* __KERNEL__ */

Comments

Single-line comments should have the leading /* and trailing */ on the same line as the comment. Multi-line comments should have the leading /* and trailing */ on their own lines, to match the upstream kernel coding style. Intermediate lines should start with a * aligned with the * on the first line:

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

External function declarations absolutely should *NEVER* go into .c files. The only exception is forward declarations for static functions in the same file that can't otherwise be moved before the caller. Instead, the declaration should go into the most "local" header available (e.g. subsystem_internal.h for a given subdirectory). Having external function declarations in .c files can cause very difficult to diagnose runtime bugs, because the compiler takes the local declaration as correct, can not compare it to the actual function declared in a different file, and does not have a declaration in a header to compare it against, but the linker does not check that the number and type of function arguments match.

Function declarations in header files should include the variable names for the parameters, so that they are self explanatory in the header without having to look at the code to see what the parameter is:

right:
    void ldlm_lock_addref_internal(struct ldlm_lock *lock, enum ldlm_lock_mode lock_mode, int refcount, int rc);

wrong:
    void ldlm_lock_addref_internal(struct ldlm_lock *, int, int, int)

Structure fields should have a common prefix derived from the structure name, so that they are easily found in the code and tag-jump works properly. This avoids problems with generic field names like page or count that have dozens or hundreds of potential matches in the code.

Structure and constant declarations should not be declared in multiple places. Put the struct into the most "local" header possible.

Structure initialization should be done by field names instead of using positional arguments:

struct foo {
        int     foo_val;
        int     foo_flag;
        int     foo_count;
        void   *foo_ptr;
        void   *foo_bar_ptr;
        void   *foo_baz_ptr;
};

right:
void func(void *data)
{
        struct foo fooz = { .foo_val = 1, .foo_flag = 0x20, .foo_ptr = data, .foo_baz_ptr = param };
}

wrong:
void func(void *data)
{
        /* not sure which field is being initialized, breaks (maybe silently) if structure adds a new field */
        struct foo fooz = { 1, 0x20, data, param };
}

When allocating/freeing a single struct, use D_ALLOC_PTR() for clarity:

right:
        D_ALLOC_PTR(mds_body);
        D_FREE_PTR(mds_body);

wrong:
        D_ALLOC(mds_body, sizeof(*mds_body));
        D_FREE(mds_body, sizeof(*mds_body));

ASSERTION

Assertions

Do not embed operations inside assertions. If assertions are disabled for performance reasons this code will not be executed.

...