diff --git a/doc/Pacemaker_Development/en-US/Book_Info.xml b/doc/Pacemaker_Development/en-US/Book_Info.xml index cc88a7caf7..b08acf54f3 100644 --- a/doc/Pacemaker_Development/en-US/Book_Info.xml +++ b/doc/Pacemaker_Development/en-US/Book_Info.xml @@ -1,36 +1,35 @@ %BOOK_ENTITIES; ]> - Pacemaker Development - Working with the Pacemaker Code Base - - 2 - 0 - - - This document has guidelines and tips for developers - interested in editing Pacemaker source code and - submitting changes for inclusion in the project. - - - - - - - - - - - + Pacemaker Development + Working with the Pacemaker Code Base + + 2 + 2 + + + This document has guidelines and tips for developers + interested in editing Pacemaker source code and + submitting changes for inclusion in the project. + + + + + + + + + + + - diff --git a/doc/Pacemaker_Development/en-US/Ch-Coding.txt b/doc/Pacemaker_Development/en-US/Ch-Coding.txt index c0bfde984c..6a54a7db0a 100644 --- a/doc/Pacemaker_Development/en-US/Ch-Coding.txt +++ b/doc/Pacemaker_Development/en-US/Ch-Coding.txt @@ -1,199 +1,296 @@ :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 2, C Coding Guidelines] -== C Boilerplate == +== 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 like this: +Every C file should start with a short copyright notice listing the original +author, like: ==== [source,C] ---- /* * Copyright Andrew Beekhof * * This source code is licensed under WITHOUT ANY WARRANTY. */ ---- ==== -++ is the year the code was 'originally' created. +The first ++ is the year the code was 'originally' created. footnote:[ See the U.S. Copyright Office's https://www.copyright.gov/comp3/["Compendium of U.S. Copyright Office Practices"], particularly "Chapter 2200: Notice of Copyright", sections 2205.1(A) and 2205.1(F), or https://techwhirl.com/updating-copyright-notices/["Updating Copyright Notices"] for a more readable summary. ] If the code is modified in later years, add +-YYYY+ with the most recent year of modification. ++ 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+)". -== Formatting == +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 +---- +==== -=== Whitespace === +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. - -=== Line Length === - - 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; ---- ==== -=== Functions === +=== 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: - -==== -[source,C] ----- -static int -foo(void) -{ ----- -==== - + 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: + 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, const char *d) + 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: + 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] ---- -if (condition1) { +a = f(); +if (a < 0) { statement1; -} else if (condition2) { +} 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. +- 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); ---- ==== -== Naming Conventions == +== 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 symols 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_+. + +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 == +=== 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 ---- diff --git a/doc/Pacemaker_Development/en-US/Revision_History.xml b/doc/Pacemaker_Development/en-US/Revision_History.xml index 594b5df66d..6769c833ea 100644 --- a/doc/Pacemaker_Development/en-US/Revision_History.xml +++ b/doc/Pacemaker_Development/en-US/Revision_History.xml @@ -1,65 +1,77 @@ %BOOK_ENTITIES; ]> Revision History 1-0 Tue Jul 26 2016 KenGaillot kgaillot@redhat.com Convert coding guidelines and developer FAQ to Publican document 1-1 Mon Aug 29 2016 KenGaillot kgaillot@redhat.com Add Python coding guidelines, and more about licensing 2-0 Fri Jan 12 2018 KenGaillot kgaillot@redhat.com Drop support for Python 2.6 2-1 Tue Sep 18 2018 JanPokorný poki@redhat.com Start documenting notable evolutionary points + + 2-2 + Fri Dec 7 2018 + + KenGaillot + kgaillot@redhat.com + + + Update FAQ and C guidelines + + +