diff --git a/doc/Pacemaker_Development/en-US/Ch-Coding.txt b/doc/Pacemaker_Development/en-US/Ch-Coding.txt index 2e6d0eb010..bfc003b780 100644 --- a/doc/Pacemaker_Development/en-US/Ch-Coding.txt +++ b/doc/Pacemaker_Development/en-US/Ch-Coding.txt @@ -1,286 +1,291 @@ :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 the Pacemaker project contributors * * The version control history for this file may have further details. * * This source code is licensed under WITHOUT ANY WARRANTY. */ ---- ==== ++ 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)+. +- Be sure 'not' to pass +NULL+ as an argument to satisfy +%s+ format specifier + in logging and more generally +printf+ style functions. When mere ++ + string is a sufficient output representation in such case, there is + +crm_str+ convenient macro. Ternary operator is an apparent choice otherwise. + === 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 ----