diff options
Diffstat (limited to 'doc/sphinx/Pacemaker_Development/c.rst')
| -rw-r--r-- | doc/sphinx/Pacemaker_Development/c.rst | 955 |
1 files changed, 955 insertions, 0 deletions
diff --git a/doc/sphinx/Pacemaker_Development/c.rst b/doc/sphinx/Pacemaker_Development/c.rst new file mode 100644 index 0000000..66ce3b2 --- /dev/null +++ b/doc/sphinx/Pacemaker_Development/c.rst @@ -0,0 +1,955 @@ +.. index:: + single: C + pair: C; guidelines + +C Coding Guidelines +------------------- + +Pacemaker is a large 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. Following consistent guidelines +makes reading, writing, and reviewing code easier, and helps avoid common +mistakes. + +Some existing Pacemaker code does not follow these guidelines, for historical +reasons and API backward compatibility, but new code should. + + +Code Organization +################# + +Pacemaker's C code is organized as follows: + ++-----------------+-----------------------------------------------------------+ +| Directory | Contents | ++=================+===========================================================+ +| daemons | the Pacemaker daemons (pacemakerd, pacemaker-based, etc.) | ++-----------------+-----------------------------------------------------------+ +| include | header files for library APIs | ++-----------------+-----------------------------------------------------------+ +| lib | libraries | ++-----------------+-----------------------------------------------------------+ +| tools | command-line tools | ++-----------------+-----------------------------------------------------------+ + +Source file names should be unique across the entire project, to allow for +individual tracing via ``PCMK_trace_files``. + + +.. index:: + single: C; library + single: C library + +Pacemaker Libraries +################### + ++---------------+---------+---------------+---------------------------+-------------------------------------+ +| Library | Symbol | Source | API Headers | Description | +| | prefix | location | | | ++===============+=========+===============+===========================+=====================================+ +| libcib | cib | lib/cib | | include/crm/cib.h | .. index:: | +| | | | | include/crm/cib/* | single: C library; libcib | +| | | | | single: libcib | +| | | | | | +| | | | | API for pacemaker-based IPC and | +| | | | | the CIB | ++---------------+---------+---------------+---------------------------+-------------------------------------+ +| libcrmcluster | pcmk | lib/cluster | | include/crm/cluster.h | .. index:: | +| | | | | include/crm/cluster/* | single: C library; libcrmcluster | +| | | | | single: libcrmcluster | +| | | | | | +| | | | | Abstract interface to underlying | +| | | | | cluster layer | ++---------------+---------+---------------+---------------------------+-------------------------------------+ +| libcrmcommon | pcmk | lib/common | | include/crm/common/* | .. index:: | +| | | | | some of include/crm/* | single: C library; libcrmcommon | +| | | | | single: libcrmcommon | +| | | | | | +| | | | | Everything else | ++---------------+---------+---------------+---------------------------+-------------------------------------+ +| libcrmservice | svc | lib/services | | include/crm/services.h | .. index:: | +| | | | | single: C library; libcrmservice | +| | | | | single: libcrmservice | +| | | | | | +| | | | | Abstract interface to supported | +| | | | | resource types (OCF, LSB, etc.) | ++---------------+---------+---------------+---------------------------+-------------------------------------+ +| liblrmd | lrmd | lib/lrmd | | include/crm/lrmd*.h | .. index:: | +| | | | | single: C library; liblrmd | +| | | | | single: liblrmd | +| | | | | | +| | | | | API for pacemaker-execd IPC | ++---------------+---------+---------------+---------------------------+-------------------------------------+ +| libpacemaker | pcmk | lib/pacemaker | | include/pacemaker*.h | .. index:: | +| | | | | include/pcmki/* | single: C library; libpacemaker | +| | | | | single: libpacemaker | +| | | | | | +| | | | | High-level APIs equivalent to | +| | | | | command-line tool capabilities | +| | | | | (and high-level internal APIs) | ++---------------+---------+---------------+---------------------------+-------------------------------------+ +| libpe_rules | pe | lib/pengine | | include/crm/pengine/* | .. index:: | +| | | | | single: C library; libpe_rules | +| | | | | single: libpe_rules | +| | | | | | +| | | | | Scheduler functionality related | +| | | | | to evaluating rules | ++---------------+---------+---------------+---------------------------+-------------------------------------+ +| libpe_status | pe | lib/pengine | | include/crm/pengine/* | .. index:: | +| | | | | single: C library; libpe_status | +| | | | | single: libpe_status | +| | | | | | +| | | | | Low-level scheduler functionality | ++---------------+---------+---------------+---------------------------+-------------------------------------+ +| libstonithd | stonith | lib/fencing | | include/crm/stonith-ng.h| .. index:: | +| | | | | include/crm/fencing/* | single: C library; libstonithd | +| | | | | single: libstonithd | +| | | | | | +| | | | | API for pacemaker-fenced IPC | ++---------------+---------+---------------+---------------------------+-------------------------------------+ + + +Public versus Internal APIs +___________________________ + +Pacemaker libraries have both internal and public APIs. Internal APIs are those +used only within Pacemaker; public APIs are those offered (via header files and +documentation) for external code to use. + +Generic functionality needed by Pacemaker itself, such as string processing or +XML processing, should remain internal, while functions providing useful +high-level access to Pacemaker capabilities should be public. When in doubt, +keep APIs internal, because it's easier to expose a previously internal API +than hide a previously public API. + +Internal APIs can be changed as needed. + +The public API/ABI should maintain a degree of stability so that external +applications using it do not need to be rewritten or rebuilt frequently. Many +OSes/distributions avoid breaking API/ABI compatibility within a major release, +so if Pacemaker breaks compatibility, that significantly delays when OSes +can package the new version. Therefore, changes to public APIs should be +backward-compatible (as detailed throughout this chapter), unless we are doing +a (rare) release where we specifically intend to break compatibility. + +External applications known to use Pacemaker's public C API include +`sbd <https://github.com/ClusterLabs/sbd>`_ and dlm_controld. + + +.. index:: + pair: C; naming + +API Symbol Naming +_________________ + +Exposed API symbols (non-``static`` function names, ``struct`` and ``typedef`` +names in header files, etc.) must begin with the prefix appropriate to the +library (shown in the table at the beginning of this section). This reduces the +chance of naming collisions when external software links against the library. + +The prefix is usually lowercase but may be all-caps for some defined constants +and macros. + +Public API symbols should follow the library prefix with a single underbar +(for example, ``pcmk_something``), and internal API symbols with a double +underbar (for example, ``pcmk__other_thing``). + +File-local symbols (such as static functions) and non-library code do not +require a prefix, though a unique prefix indicating an executable (controld, +crm_mon, etc.) can be helpful when symbols are shared between multiple +source files for the executable. + + +API Header File Naming +______________________ + +* Internal API headers should be named ending in ``_internal.h``, in the same + location as public headers, with the exception of libpacemaker, which for + historical reasons keeps internal headers in ``include/pcmki/pcmki_*.h``). + +* If a library needs to share symbols just within the library, header files for + these should be named ending in ``_private.h`` and located in the library + source directory (not ``include``). Such functions should be declared as + ``G_GNUC_INTERNAL``, to aid compiler efficiency (glib defines this + symbol appropriately for the compiler). + +Header files that are not library API are kept in the same directory as the +source code they're included from. + +The easiest way to tell what kind of API a symbol is, is to see where it's +declared. If it's in a public header, it's public API; if it's in an internal +header, it's internal API; if it's in a library-private header, it's +library-private API; otherwise, it's not an API. + + +.. index:: + pair: C; API documentation + single: Doxygen + +API Documentation +_________________ + +Pacemaker uses `Doxygen <https://www.doxygen.nl/manual/docblocks.html>`_ +to automatically generate its +`online API documentation <https://clusterlabs.org/pacemaker/doxygen/>`_, +so all public API (header files, functions, structs, enums, etc.) should be +documented with Doxygen comment blocks. Other code may be documented in the +same way if desired, with an ``\internal`` tag in the Doxygen comment. + +Simple example of an internal function with a Doxygen comment block: + +.. code-block:: c + + /*! + * \internal + * \brief Return string length plus 1 + * + * Return the number of characters in a given string, plus one. + * + * \param[in] s A string (must not be NULL) + * + * \return The length of \p s plus 1. + */ + static int + f(const char *s) + { + return strlen(s) + 1; + } + +Function arguments are marked as ``[in]`` for input only, ``[out]`` for output +only, or ``[in,out]`` for both input and output. + +``[in,out]`` should be used for struct pointer arguments if the function can +change any data accessed via the pointer. For example, if the struct contains +a ``GHashTable *`` member, the argument should be marked as ``[in,out]`` if the +function inserts data into the table, even if the struct members themselves are +not changed. However, an argument is not ``[in,out]`` if something reachable +via the argument is modified via a separate argument. For example, both +``pe_resource_t`` and ``pe_node_t`` contain pointers to their +``pe_working_set_t`` and thus indirectly to each other, but if the function +modifies the resource via the resource argument, the node argument does not +have to be ``[in,out]``. + + +Public API Deprecation +______________________ + +Public APIs may not be removed in most Pacemaker releases, but they may be +deprecated. + +When a public API is deprecated, it is moved to a header whose name ends in +``compat.h``. The original header includes the compatibility header only if the +``PCMK_ALLOW_DEPRECATED`` symbol is undefined or defined to 1. This allows +external code to continue using the deprecated APIs, but internal code is +prevented from using them because the ``crm_internal.h`` header defines the +symbol to 0. + + +.. index:: + pair: C; boilerplate + pair: license; C + pair: copyright; C + +C Boilerplate +############# + +Every C file should start with a short copyright and license notice: + +.. code-block:: 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 +`COPYING <https://github.com/ClusterLabs/pacemaker/blob/main/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 of the form ``PCMK__<capitalized_header_name>__H``. For +example: + +.. code-block:: c + + #ifndef PCMK__MY_HEADER__H + # define PCMK__MY_HEADER__H + + // header code here + + #endif // PCMK__MY_HEADER__H + +Public API header files should additionally declare "C" compatibility for +inclusion by C++, and give a Doxygen file description. For example: + +.. code-block:: c + + #ifdef __cplusplus + extern "C" { + #endif + + /*! + * \file + * \brief My brief description here + * \ingroup core + */ + + // header code here + + #ifdef __cplusplus + } + #endif + + +.. index:: + pair: C; whitespace + +Line Formatting +############### + +* Indentation must be 4 spaces, no tabs. + +* Do not leave trailing whitespace. + +* Lines should be no longer than 80 characters unless limiting line length + hurts readability. + + +.. index:: + pair: C; comment + +Comments +######## + +.. code-block:: c + + /* Single-line comments may look like this */ + + // ... or this + + /* Multi-line comments should start immediately after the comment opening. + * Subsequent lines should start with an aligned asterisk. The comment + * closing should be aligned and on a line by itself. + */ + + +.. index:: + pair: C; operator + +Operators +######### + +.. code-block:: c + + // Operators have spaces on both sides + x = a; + + /* (1) Do not rely on operator precedence; use parentheses when mixing + * operators with different priority, for readability. + * (2) No space is used after an opening parenthesis or before a closing + * parenthesis. + */ + x = a + b - (c * d); + + +.. index:: + single: C; if + single: C; else + single: C; while + single: C; for + single: C; switch + +Control Statements (if, else, while, for, switch) +################################################# + +.. code-block:: c + + /* + * (1) The control keyword is followed by a space, a left parenthesis + * without a space, the condition, a right parenthesis, a space, and the + * opening bracket on the same line. + * (2) 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 the chance of bad indenting making a + * line incorrectly appear to be part of the block. + * (3) The closing bracket is on a line by itself. + */ + if (v < 0) { + return 0; + } + + /* "else" and "else if" are on the same line with the previous ending brace + * and next opening brace, separated by a space. Blank lines may be used + * between blocks to help readability. + */ + if (v > 0) { + return 0; + + } else if (a == 0) { + return 1; + + } else { + return 2; + } + + /* Do not use assignments in conditions. This ensures that the developer's + * intent is always clear, makes code reviews easier, and reduces the chance + * of using assignment where comparison is intended. + */ + // Do this ... + a = f(); + if (a) { + return 0; + } + // ... NOT this + if (a = f()) { + return 0; + } + + /* It helps readability to use the "!" operator only in boolean + * comparisons, and explicitly compare numeric values against 0, + * pointers against NULL, etc. This helps remind the reader of the + * type being compared. + */ + int i = 0; + char *s = NULL; + bool cond = false; + + if (!cond) { + return 0; + } + if (i == 0) { + return 0; + } + if (s == NULL) { + return 0; + } + + /* In a "switch" statement, indent "case" one level, and indent the body of + * each "case" another level. + */ + switch (expression) { + case 0: + command1; + break; + case 1: + command2; + break; + default: + command3; + break; + } + + +.. index:: + pair: C; macro + +Macros +###### + +Macros are a powerful but easily misused feature of the C preprocessor, and +Pacemaker uses a lot of obscure macro features. If you need to brush up, the +`GCC documentation for macros +<https://gcc.gnu.org/onlinedocs/cpp/Macros.html#Macros>`_ is excellent. + +Some common issues: + +* Beware of side effects in macro arguments that may be evaluated more than + once +* Always parenthesize macro arguments used in the macro body to avoid + precedence issues if the argument is an expression +* Multi-statement macro bodies should be enclosed in do...while(0) to make them + behave more like a single statement and avoid control flow issues + +Often, a static inline function defined in a header is preferable to a macro, +to avoid the numerous issues that plague macros and gain the benefit of +argument and return value type checking. + + +.. index:: + pair: C; memory + +Memory Management +################# + +* Always use ``calloc()`` rather than ``malloc()``. It has no additional cost on + modern operating systems, and reduces the severity and security risks of + uninitialized memory usage bugs. + +* Ensure that all dynamically allocated memory is freed when no longer needed, + and not used after it is freed. This can be challenging in the more + event-driven, callback-oriented sections of code. + +* Free dynamically allocated memory using the free function corresponding to + how it was allocated. For example, use ``free()`` with ``calloc()``, and + ``g_free()`` with most glib functions that allocate objects. + + +.. index:: + single: C; struct + +Structures +########## + +Changes to structures defined in public API headers (adding or removing +members, or changing member types) are generally not possible without breaking +API compatibility. However, there are exceptions: + +* Public API structures can be designed such that they can be allocated only + via API functions, not declared directly or allocated with standard memory + functions using ``sizeof``. + + * This can be enforced simply by documentating the limitation, in which case + new ``struct`` members can be added to the end of the structure without + breaking compatibility. + + * Alternatively, the structure definition can be kept in an internal header, + with only a pointer type definition kept in a public header, in which case + the structure definition can be changed however needed. + + +.. index:: + single: C; variable + +Variables +######### + +.. index:: + single: C; pointer + +Pointers +________ + +.. code-block:: c + + /* (1) The asterisk goes by the variable name, not the type; + * (2) Avoid leaving pointers uninitialized, to lessen the impact of + * use-before-assignment bugs + */ + char *my_string = NULL; + + // Use space before asterisk and after closing parenthesis in a cast + char *foo = (char *) bar; + +.. index:: + single: C; global variable + +Globals +_______ + +Global variables should be avoided in libraries when possible. State +information should instead be passed as function arguments (often as a +structure). This is not for thread safety -- Pacemaker's use of forking +ensures it will never be threaded -- but it does minimize overhead, +improve readability, and avoid obscure side effects. + +Variable Naming +_______________ + +Time intervals are sometimes represented in Pacemaker code as user-defined +text specifications (for example, "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 (for example, use ``interval_spec``, ``interval_ms``, +or ``interval_ms_s`` instead of ``interval``). + +.. index:: + pair: C; booleans + pair: C; bool + pair: C; gboolean + +Booleans +________ + +Booleans in C can be represented by an integer type, ``bool``, or ``gboolean``. + +Integers are sometimes useful for storing booleans when they must be converted +to and from a string, such as an XML attribute value (for which +``crm_element_value_int()`` can be used). Integer booleans use 0 for false and +nonzero (usually 1) for true. + +``gboolean`` should be used with glib APIs that specify it. ``gboolean`` should +always be used with glib's ``TRUE`` and ``FALSE`` constants. + +Otherwise, ``bool`` should be preferred. ``bool`` should be used with the +``true`` and ``false`` constants from the ``stdbool.h`` header. + +Do not use equality operators when testing booleans. For example: + +.. code-block:: c + + // Do this + if (bool1) { + fn(); + } + if (!bool2) { + fn2(); + } + + // Not this + if (bool1 == true) { + fn(); + } + if (bool2 == false) { + fn2(); + } + + // Otherwise there's no logical end ... + if ((bool1 == false) == true) { + fn(); + } + + +.. index:: + pair: C; strings + +String Handling +############### + +Define Constants for Magic Strings +__________________________________ + +A "magic" string is one used for control purposes rather than human reading, +and which must be exactly the same every time it is used. Examples would be +configuration option names, XML attribute names, or environment variable names. + +These should always be defined constants, rather than using the string literal +everywhere. If someone mistypes a defined constant, the code won't compile, but +if they mistype a literal, it could go unnoticed until a user runs into a +problem. + + +String-Related Library Functions +________________________________ + +Pacemaker's libcrmcommon has a large number of functions to assist in string +handling. The most commonly used ones are: + +* ``pcmk__str_eq()`` tests string equality (similar to ``strcmp()``), but can + handle NULL, and takes options for case-insensitive, whether NULL should be + considered a match, etc. +* ``crm_strdup_printf()`` takes ``printf()``-style arguments and creates a + string from them (dynamically allocated, so it must be freed with + ``free()``). It asserts on memory failure, so the return value is always + non-NULL. + +String handling functions should almost always be internal API, since Pacemaker +isn't intended to be used as a general-purpose library. Most are declared in +``include/crm/common/strings_internal.h``. ``util.h`` has some older ones that +are public API (for now, but will eventually be made internal). + +char*, gchar*, and GString +__________________________ + +When using dynamically allocated strings, be careful to always use the +appropriate free function. + +* ``char*`` strings allocated with something like ``calloc()`` must be freed + with ``free()``. Most Pacemaker library functions that allocate strings use + this implementation. +* glib functions often use ``gchar*`` instead, which must be freed with + ``g_free()``. +* Occasionally, it's convenient to use glib's flexible ``GString*`` type, which + must be freed with ``g_string_free()``. + +.. index:: + pair: C; regular expression + +Regular Expressions +___________________ + +- Use ``REG_NOSUB`` with ``regcomp()`` whenever possible, for efficiency. +- Be sure to use ``regfree()`` appropriately. + + +.. index:: + single: C; enum + +Enumerations +############ + +* Enumerations should not have a ``typedef``, and do not require any naming + convention beyond what applies to all exposed symbols. + +* New values should usually be added to the end of public API enumerations, + because the compiler will define the values to 0, 1, etc., in the order + given, and inserting a value in the middle would change the numerical values + of all later values, breaking code compiled with the old values. However, if + enum numerical values are explicitly specified rather than left to the + compiler, new values can be added anywhere. + +* When defining constant integer values, enum should be preferred over + ``#define`` or ``const`` when possible. This allows type checking without + consuming memory. + +Flag groups +___________ + +Pacemaker often uses flag groups (also called bit fields or bitmasks) for a +collection of boolean options (flags/bits). + +This is more efficient for storage and manipulation than individual booleans, +but its main advantage is when used in public APIs, because using another bit +in a bitmask is backward compatible, whereas adding a new function argument (or +sometimes even a structure member) is not. + +.. code-block:: c + + #include <stdint.h> + + /* (1) Define an enumeration to name the individual flags, for readability. + * An enumeration is preferred to a series of "#define" constants + * because it is typed, and logically groups the related names. + * (2) Define the values using left-shifting, which is more readable and + * less error-prone than hexadecimal literals (0x0001, 0x0002, 0x0004, + * etc.). + * (3) Using a comma after the last entry makes diffs smaller for reviewing + * if a new value needs to be added or removed later. + */ + enum pcmk__some_bitmask_type { + pcmk__some_value = (1 << 0), + pcmk__other_value = (1 << 1), + pcmk__another_value = (1 << 2), + }; + + /* The flag group itself should be an unsigned type from stdint.h (not + * the enum type, since it will be a mask of the enum values and not just + * one of them). uint32_t is the most common, since we rarely need more than + * 32 flags, but a smaller or larger type could be appropriate in some + * cases. + */ + uint32_t flags = pcmk__some_value|pcmk__other_value; + + /* If the values will be used only with uint64_t, define them accordingly, + * to make compilers happier. + */ + enum pcmk__something_else { + pcmk__whatever = (UINT64_C(1) << 0), + }; + +We have convenience functions for checking flags (see ``pcmk_any_flags_set()``, +``pcmk_all_flags_set()``, and ``pcmk_is_set()``) as well as setting and +clearing them (see ``pcmk__set_flags_as()`` and ``pcmk__clear_flags_as()``, +usually used via wrapper macros defined for specific flag groups). These +convenience functions should be preferred to direct bitwise arithmetic, for +readability and logging consistency. + + +.. index:: + pair: C; function + +Functions +######### + +Function names should be unique across the entire project, to allow for +individual tracing via ``PCMK_trace_functions``, and make it easier to search +code and follow detail logs. + + +Function Definitions +____________________ + +.. code-block:: c + + /* + * (1) The return type goes on its own line + * (2) The opening brace goes by itself on a line + * (3) Use "const" with pointer arguments whenever appropriate, to allow the + * function to be used by more callers. + */ + int + my_func1(const char *s) + { + return 0; + } + + /* Functions with no arguments must explicitly list them as void, + * for compatibility with strict compilers + */ + int + my_func2(void) + { + return 0; + } + + /* + * (1) For functions with enough arguments that they must break to the next + * line, align arguments with the first argument. + * (2) When a function argument is a function itself, use the pointer form. + * (3) Declare functions and file-global variables as ``static`` whenever + * appropriate. This gains a slight efficiency in shared libraries, and + * helps the reader know that it is not used outside the one file. + */ + static int + my_func3(int bar, const char *a, const char *b, const char *c, + void (*callback)()) + { + return 0; + } + + +Return Values +_____________ + +Functions that need to indicate success or failure should follow one of the +following guidelines. More details, including functions for using them in user +messages and converting from one to another, can be found in +``include/crm/common/results.h``. + +* A **standard Pacemaker return code** is one of the ``pcmk_rc_*`` enum values + or a system errno code, as an ``int``. + +* ``crm_exit_t`` (the ``CRM_EX_*`` enum values) is a system-independent code + suitable for the exit status of a process, or for interchange between nodes. + +* Other special-purpose status codes exist, such as ``enum ocf_exitcode`` for + the possible exit statuses of OCF resource agents (along with some + Pacemaker-specific extensions). It is usually obvious when the context calls + for such. + +* Some older Pacemaker APIs use the now-deprecated "legacy" return values of + ``pcmk_ok`` or the positive or negative value of one of the ``pcmk_err_*`` + constants or system errno codes. + +* Functions registered with external libraries (as callbacks for example) + should use the appropriate signature defined by those libraries, rather than + follow Pacemaker guidelines. + +Of course, functions may have return values that aren't success/failure +indicators, such as a pointer, integer count, or bool. + + +Public API Functions +____________________ + +Unless we are doing a (rare) release where we break public API compatibility, +new public API functions can be added, but existing function signatures (return +type, name, and argument types) should not be changed. To work around this, an +existing function can become a wrapper for a new function. + + +.. index:: + pair: C; logging + pair: C; output + +Logging and Output +################## + +Logging Vs. Output +__________________ + +Log messages and output messages are logically similar but distinct. +Oversimplifying a bit, daemons log, and tools output. + +Log messages are intended to help with troubleshooting and debugging. +They may have a high level of technical detail, and are usually filtered by +severity -- for example, the system log by default gets messages of notice +level and higher. + +Output is intended to let the user know what a tool is doing, and is generally +terser and less technical, and may even be parsed by scripts. Output might have +"verbose" and "quiet" modes, but it is not filtered by severity. + +Common Guidelines for All Messages +__________________________________ + +* 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``. + +* Do not rely on ``%s`` handling ``NULL`` values properly. While the standard + library functions might, not all functions using printf-style formatting + does, and it's safest to get in the habit of always ensuring format values + are non-NULL. If a value can be NULL, the ``pcmk__s()`` function is a + convenient way to say "this string if not NULL otherwise this default". + +* The convenience macros ``pcmk__plural_s()`` and ``pcmk__plural_alt()`` are + handy when logging a word that may be singular or plural. + +Logging +_______ + +Pacemaker uses libqb for logging, but wraps it with a higher level of +functionality (see ``include/crm/common/logging*h``). + +A few macros ``crm_err()``, ``crm_warn()``, etc. do most of the heavy lifting. + +By default, Pacemaker sends logs at notice level and higher to the system log, +and logs at info level and higher to the detail log (typically +``/var/log/pacemaker/pacemaker.log``). The intent is that most users will only +ever need the system log, but for deeper troubleshooting and developer +debugging, the detail log may be helpful, at the cost of being more technical +and difficult to follow. + +The same message can have more detail in the detail log than in the system log, +using libqb's "extended logging" feature: + +.. code-block:: c + + /* The following will log a simple message in the system log, like: + + warning: Action failed: Node not found + + with extra detail in the detail log, like: + + warning: Action failed: Node not found | rc=-1005 id=hgjjg-51006 + */ + crm_warn("Action failed: %s " CRM_XS " rc=%d id=%s", + pcmk_rc_str(rc), rc, id); + + +Output +______ + +Pacemaker has a somewhat complicated system for tool output. The main benefit +is that the user can select the output format with the ``--output-as`` option +(usually "text" for human-friendly output or "xml" for reliably script-parsable +output, though ``crm_mon`` additionally supports "console" and "html"). + +A custom message can be defined with a unique string identifier, plus +implementation functions for each supported format. The caller invokes the +message using the identifier. The user selects the output format via +``--output-as``, and the output code automatically calls the appropriate +implementation function. + +The interface (most importantly ``pcmk__output_t``) is declared in +``include/crm/common/output*h``. See the API comments and existing tools for +examples. + + +.. index:: + single: Makefile.am + +Makefiles +######### + +Pacemaker uses +`automake <https://www.gnu.org/software/automake/manual/automake.html>`_ +for building, so the Makefile.am in each directory should be edited rather than +Makefile.in or Makefile, which are automatically generated. + +* Public API headers are installed (by adding them to a ``HEADERS`` variable in + ``Makefile.am``), but internal API headers are not (by adding them to + ``noinst_HEADERS``). + + +.. index:: + pair: C; vim settings + +vim Settings +############ + +Developers who use ``vim`` to edit source code can add the following settings +to their ``~/.vimrc`` file to follow Pacemaker C coding guidelines: + +.. code-block:: none + + " 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 |