dhodovsk / source-git / pacemaker

Forked from source-git/pacemaker 3 years ago
Clone
Blob Blame History Raw
:compat-mode: legacy
= C Coding Guidelines =

////
We prefer [[ch-NAME]], but older versions of asciidoc don't deal well
with that construct for chapter headings
////
anchor:ch-c-coding[Chapter 3, C Coding Guidelines]

== Style Guidelines ==

Pacemaker is a large, distributed project accepting contributions from
developers with a wide range of skill levels and organizational affiliations,
and maintained by multiple people over long periods of time. The guidelines in
this section are not technically better than alternative approaches, but make
project management easier.

Many of these simply ensure stylistic consistency, which makes reading,
writing, and reviewing code easier.

=== C Boilerplate ===

indexterm:[C,boilerplate]
indexterm:[licensing,C boilerplate]

Every C file should start with a short copyright notice:

====
[source,C]
----
/*
 * Copyright <YYYY[-YYYY]> the Pacemaker project contributors
 * 
 * The version control history for this file may have further details.
 *
 * This source code is licensed under <LICENSE> WITHOUT ANY WARRANTY.
 */
----
====

+<LICENSE>+ should follow the policy set forth in the
https://github.com/ClusterLabs/pacemaker/blob/master/COPYING[+COPYING+] file,
generally one of "GNU General Public License version 2 or later (GPLv2+)"
or "GNU Lesser General Public License version 2.1 or later (LGPLv2.1+)".

Header files should additionally protect against multiple inclusion by defining
a unique symbol.

====
[source,C]
----
#ifndef MY_HEADER_NAME__H
#  define MY_HEADER_NAME__H

// header code here

#endif // MY_HEADER_NAME__H
----
====

Public API header files should additionally declare "C" compatibility for
inclusion by C++, and give a Doxygen file description. For example:

====
[source,C]
----
#ifdef __cplusplus
extern "C" {
#endif

/*!
 * \file
 * \brief My brief description here
 * \ingroup core
 */

// header code here

#ifdef __cplusplus
}
#endif
----
====

=== Line Formatting ===

indexterm:[C,whitespace]

- Indentation must be 4 spaces, no tabs.
- Do not leave trailing whitespace.
- Lines should be no longer than 80 characters unless limiting line length
  significantly impacts readability.

=== Pointers ===

indexterm:[C,pointers]

- The +*+ goes by the variable name, not the type:

====
[source,C]
----
char *foo;
----
====

- Use a space before the +*+ and after the closing parenthesis in a cast:

====
[source,C]
----
char *foo = (char *) bar;
----
====

=== Function Definitions ===

indexterm:[C,functions]

- In the function definition, put the return type on its own line, and place
  the opening brace by itself on a line.
- For functions with enough arguments that they must break to the next line,
  align arguments with the first argument.
- When a function argument is a function itself, use the pointer form.

====
[source,C]
----
static int
function_name(int bar, const char *a, const char *b,
              const char *c, void (*d)())
{
----
====

- If a function name gets really long, start the arguments on their own line
  with 8 spaces of indentation:

====
[source,C]
----
static int
really_really_long_function_name_this_is_getting_silly_now(
        int bar, const char *a, const char *b,
        const char *c, const char *d)
{
----
====

=== Control Statements (if, else, while, for, switch) ===

- The keyword is followed by one space, then left parenthesis without space,
  condition, right parenthesis, space, opening bracket on the same line.
  +else+ and +else if+ are on the same line with the ending brace and opening
  brace, separated by a space.
- Always use braces around control statement blocks, even if they only contain
  one line. This makes code review diffs smaller if a line gets added in the
  future, and avoids any chance of bad indenting making a line incorrectly
  appear to be part of the block.
- Do not put assignments in +if+ or +while+ conditionals. This ensures that the
  developer's intent is always clear, making code reviews easier and reducing
  the chance of using assignment where comparison is intended.

====
[source,C]
----
a = f();
if (a < 0) {
    statement1;
} else if (some_other_condition) {
    statement2;
} else {
    statement3;
}
----
====

- In a +switch+ statement, +case+ is indented one level, and the body of each
  +case+ is indented by another level. The opening brace is on the same line as
  +switch+.

====
[source,C]
----
switch (expression) {
    case 0:
        command1;
        break;
    case 1:
        command2;
        break;
    default:
        command3;
}
----
====

=== Operators ===

indexterm:[C,operators]

- Operators have spaces from both sides.
- Do not rely on operator precedence; use parentheses when mixing operators
  with different priority.
- No space is used after opening parenthesis and before closing parenthesis.

====
[source,C]
----
x = a + b - (c * d);
----
====

== Best Practices ==

The guidelines in this section offer technical advantages.

=== New Struct and Enum Members ===

In the public APIs, always add new struct members to the end of the struct.
This allows us to maintain backward API/ABI compatibility (as long as the
application being linked allocates structs via API functions).

This generally applies to enum values as well, as the compiler will define
enum values to 0, 1, etc., in the order given, so inserting a value in the
middle will change the numerical values of all later values, making them
backward-incompatible. However, if enum numerical values are explicitly
specified rather than left to the compiler, new values can be added anywhere.

=== Documentation ===

All public API header files, functions, structs, enums, etc.,
should be documented with Doxygen comment blocks, as Pacemaker's
http://clusterlabs.org/pacemaker/doxygen/[online API documentation]
is automatically generated via Doxygen. It is helpful to document
private symbols in the same way, with an +\internal+ tag in the
Doxygen comment.

=== Symbol Naming ===

indexterm:[C,naming]

- All file and function names should be unique across the entire project,
  to allow for individual tracing via +PCMK_trace_files+ and
  +PCMK_trace_functions+, as well as making detail logs easier to follow.
- Any exposed symbols in libraries (non-+static+ function names, type names,
  etc.) must begin with a prefix appropriate to the library, for example,
  +crm_+, +pe_+, +st_+, +lrm_+. This reduces the chance of naming collisions
  with software linked against the library.
- Time intervals are sometimes represented in Pacemaker code as user-defined
  text specifications (e.g. "10s"), other times as an integer number of
  seconds or milliseconds, and still other times as a string representation
  of an integer number. Variables for these should be named with an indication
  of which is being used (e.g. +interval_spec+, +interval_ms+, or
  +interval_ms_s+ instead of +interval+).

=== Memory Allocation ===

Always use calloc() rather than malloc(). It has no additional cost on modern
operating systems, and reduces the severity of uninitialized memory usage bugs.

=== Logging ===

- When format strings are used for derived data types whose implementation may
  vary across platforms (+pid_t+, +time_t+, etc.), the safest approach is to
  use +%lld+ in the format string, and cast the value to +(long long)+.

=== Regular Expressions ===

- Use +REG_NOSUB+ with +regcomp()+ whenever possible, for efficiency.
- Be sure to use +regfree()+ appropriately.

=== vim Settings ===

indexterm:[vim]

Developers who use +vim+ to edit source code can add the following settings to
their +~/.vimrc+ file to follow Pacemaker C coding guidelines:

----
" follow Pacemaker coding guidelines when editing C source code files
filetype plugin indent on
au FileType c   setlocal expandtab tabstop=4 softtabstop=4 shiftwidth=4 textwidth=80
autocmd BufNewFile,BufRead *.h set filetype=c
let c_space_errors = 1
----