Table of Contents

C coding guidelines

C coding guidelines

Standards

Even system and library functions that conform to POSIX.1-2001 cannot be used without checking (e.g, see ZBX-3937).

Formatting

Lines should not be longer than 120 characters.
Two or more consecutive empty lines should not be used.
Variable declarations should be followed by an empty line to separate them from the rest of the code:

struct utsname  name;
       
       if (-1 == uname(&name))
               return SYSINFO_RET_FAIL;

Code that sets up the context for if and while constructs should be separated from if and while by a blank line, unless the body of if and while consists of a single statement:

package = strtok(buf, "\n");
       
       while (NULL != package)
       {
           ...
       
           package = strtok(NULL, "\n");
       }

goto labels should be written on a separate line, with no blank lines before and after:

    res = SUCCEED;
       clean:
           zabbix_log(LOG_LEVEL_DEBUG, "End of %s():%s", __function_name, zbx_result_string(res));

When type casting, no space should be added after the type:

*lastlogsize = (long)buf.st_size;

Error messages should use "cannot" instead of "can't" or "could not":

zabbix_log(LOG_LEVEL_WARNING, "cannot remove shared memory for collector [%s]", strerror(errno));

Hexadecimal constants should be written in lowercase:

#define MEM_MAX_SIZE    0x7fffffff

If a function is referenced in a comment, it should have parentheses after its name:

error = zbx_sock_last_error();  /* zabbix_log() resets the error code */

Debugging information containing a single sentence should not be capitalized and should not end with a period:

zabbix_log(LOG_LEVEL_CRIT, "failed assumption about pointer size (%lu not in {4, 8})", ZBX_PTR_SIZE);

Structure declarations should begin with typedef struct, followed by the structure body and type name zbx_<struct_name>_t:

typedef struct
       {
       }
       zbx_data_t;

If needed for forward declarations the structure name should match the type name without _t suffix - zbx_<struct_name>:

typedef struct zbx_data
       {
       }
       zbx_data_t;

Structure members should be separated by spaces, not tabs when initializing arrays of structures:

static zbx_vmcheck_t    vmchecks[] =
       {
           {"cluster.discovery", VMCHECK_FUNC(check_vcenter_cluster_discovery)},
           {"cluster.status", VMCHECK_FUNC(check_vcenter_cluster_status)},
           {"version", VMCHECK_FUNC(check_vcenter_version)},
           {NULL, NULL}
       };

There must be an empty line before return statement at the end of the function, unless it is the only line of function body:

static int      proc_get_process_uid(pid_t pid, uid_t *uid)
       {
               char            tmp[MAX_STRING_LEN];
               zbx_stat_t      st;
       
               zbx_snprintf(tmp, sizeof(tmp), "/proc/%d", (int)pid);
       
               if (0 != zbx_stat(tmp, &st))
                       return FAIL;
       
               *uid = st.st_uid;
       
               return SUCCEED;
       }

Short enough label can be treated as empty string:

static int      smtp_debug_function(CURL *easyhandle, curl_infotype type, char *data, size_t size, void *userptr)
       {
               const char      labels[3] = {'*', '<', '>'};
       
               if (CURLINFO_TEXT != type && CURLINFO_HEADER_IN != type && CURLINFO_HEADER_OUT != type)
                       goto out;
       
               while (0 < size && ('\r' == data[size - 1] || '\n' == data[size - 1]))
                       size--;
       
               zabbix_log(LOG_LEVEL_TRACE, "%c %.*s", labels[type], (int)size, data);
       out:
               return 0;
       }

Naming

Variables and functions should be named with underscores between words:

int calculate_item_nextcheck(zbx_uint64_t interfaceid, zbx_uint64_t itemid, int item_type,
               int delay, const char *flex_intervals, time_t now, int *effective_delay);

Use of camel case for naming variables in allowed only in Windows-specific code for consistency with Windows API:

PERFCOUNTER *counterName = NULL;
       DWORD       dwSize;

Public functions (declared in the "include" directory header files) must be named with prefix "zbx_".

int zbx_get_component_version(char *value);

Comments

Comments containing a single sentence should not be capitalized and should not end with a period:

/* check whether we need to update items_hk index */

Comments containing several sentences should be properly capitalized and end with a period:

/* The agent expects ALWAYS to have lastlogsize and mtime tags. Removing those would cause older agents to fail. */

Comment placement in regular code should indicate what the comment belongs to. For instance, a comment right before a single line of code or right before a block of consecutive lines indicates what this line or a block of lines does:

/* do not reallocate if not much is freed */
       if (size > chunk_size / 4)
           return chunk;

If a comment pertains to the whole block between { and }, it should be written on a separate line at the top of the block.

if (0 != next_free)
       {
           /* merge with next chunk */
       
           info→used_size -= chunk_size;
           info→used_size += size;
       
           ...
       }

If there is a comment at the end of a line after a statement, it should be separated from the statement by tabs:

free_perf_collector();  /* cpu_collector must be freed before perf_collector is freed */

In case of a multi-line comment treat every line as a single comment:

if (FAIL == result)
       {
           /* since we have successfully sent data earlier, we assume the other */
           /* side is just too busy processing our data if there is no response */
           ret = NETWORK_ERROR;
       }

Functions descriptions

If there is a need for a comment before a function, but all four parts ("Purpose", "Parameters", "Return value", "Comments") would be too much, use only the "Comments" part. Use of single-line comments before function definitions should be avoided:

/******************************************************************************
        *                                                                            *
        * Comments: counter is NULL if it is not in the collector,                   *
        *           do not call it for PERF_COUNTER_ACTIVE counters                  *
        *                                                                            *
        ******************************************************************************/
       PDH_STATUS  zbx_PdhAddCounter(const char *function, PERF_COUNTERS *counter, PDH_HQUERY query, ...

Redundant arguments descriptions are not allowed:

 * Parameters:
           data            - [IN] the data                  *  <- DEFINITELY REDUNDANT
           alerter_service - [IN] the alerter service       *  <- DEFINITELY REDUNDANT
           mediatypeid     - [IN] the media type identifier *  <- DEFINITELY REDUNDANT
           alert           - [IN] the alert object          *  <- LIKELY REDUNDANT
           manager         - [IN] the alert manager         *  <- NOT REDUNDANT
           client          - [IN] the first connected worker interprocess communication
                                   client                    *  <- NOT REDUNDANT
           result          - [OUT] the server response/error message, optional
                       If result is specified it must always be freed
                       with the first callback. *  <- NOT REDUNDANT
           error           - [OUT] the error message        *  <- technically NOT REDUNDANT, but
                       the clearer argument name would be 'error_msg'

Also, the English articles "the", "a/an" are not allowed in simple arguments descriptions (to preserve space). The exception to this are the full proper English sentences, where they are allowed (but not mandatory). The corrected descriptions:

 * Parameters:
           data            - [IN]
           alerter_service - [IN]
           mediatypeid     - [IN]
           alert           - [IN]
           manager         - [IN] alert manager
           client          - [IN] first connected worker interprocess communication
                       client
           result          - [OUT] server response/error message, optional
                       If result is specified it must always be freed
                       with the first callback.
           error_msg       - [OUT]

Capital casing must be used for acronyms:

 * Purpose: XML constructor
        * ...
        *             jp_row - [IN] discovery data for LLD macro

Not 'xml' or 'lld'.

Conditional statements

If expression is compared to a constant, the constant should be written on the left:

if (SUCCEED == str_in_list("last,prev", function, ','))

if (2 < num_param(param))

If a constant is used in a bitwise expression, it should be written on the right:

if (0x80 == (*text & 0xc0))

If a numeric value or a pointer is tested for being non-zero, write 0 or NULL explicitly:

if (NULL != message_esc && 0 != message_esc_len)

The only exception to the "constant on the left in comparison" rule is when a value is checked for being within certain bounds:

if ('1' <= *(br - 2) && *(br - 2) <= '9')

If there is an aesthetic need for parentheses in a ternary expression, put them around the whole expression, not just the conditional part:

is_numeric |= (SUCCEED == _wis_uint(cpe→szCounterName) ? 0x02 : 0);

In case conditional expression in an if takes several lines the logical operators should be written at the end of those lines and the following statement(s) should always be in braces. Also, second and further lines of the conditional expression should be indented with two tabs, not one:

if (0 == ioctl(s, SIOCGIFFLAGS, ifr) &&
               0 == (ifr→ifr_flags & IFF_LOOPBACK) &&
               0 == ioctl(s, SIOCGIFHWADDR, ifr))
       {
           ...
       }

If a conditional statement features if, else, and maybe even else if, and at least one of the branches contains multiple statements, there should be braces around each branch. The exception is the last else where braces can be omitted:

if (0 == found)
       {
           *curr = zbx_strpool_intern(new);
       }
       else if (0 != strcmp(*curr, new))
       {
           zbx_strpool_release(*curr);
           *curr = zbx_strpool_intern(new);
       }
       else
           return FAIL;

switch statement labels should be written on a separate line, too:

switch (dcheck→type)
       {
           case SVC_TELNET:
               service = "telnet";
               break;
           case SVC_ICMPPING:
               break;
           default:
               return FAIL;
       }

There should be no empty line before break in a switch statement, even in larger case blocks:

switch (value_type)
       {
           case ITEM_VALUE_TYPE_FLOAT:
               numeric = 1;
               break;
           case ITEM_VALUE_TYPE_STR:
               if (NULL == strval)
               {
                   strval = zbx_strdup(val);
                   zbx_free(val);
       
                   if ('\0' != *strval)
                   {
                       if (SUCCEED != validate(strval))
                           ret = FAIL;
                   }
                   else
                       zabbix_log(LOG_LEVEL_DEBUG, "received empty value");
               }
               break;
           default:
               return FAIL;
       }

default case is not obligatory in a switch statement:

switch (aggr_func)
       {
           case ZBX_AGGR_FUNC_ONE:
           case ZBX_AGGR_FUNC_AVG:
               total += one_total;
               counter += one_counter;
               break;
           case ZBX_AGGR_FUNC_MAX:
               if (0 == process_num || one_counter > counter)
               {
                   counter = one_counter;
                   total = one_total;
               }
               break;
       }

The break should not be used in a switch statement if it has no meaning :

switch (value_type)
       {
           case ITEM_VALUE_TYPE_FLOAT:
               return "history";
           case ITEM_VALUE_TYPE_STR:
               return "history_str";
           case ITEM_VALUE_TYPE_LOG:
               return "history_log";
           default:
               assert(0);
       }

Logging

In case log level is WARNING or less string quoting must be done using double quotes otherwise single quotes are preferred:

zabbix_log(LOG_LEVEL_WARNING, "cannot remove host \"%s\": host not found", host);
       zabbix_log(LOG_LEVEL_DEBUG, "In %s() host:'%s' hostid:%d", __function_name, host, hostid);

Vectors

The vector implementation supports various operations such as sorting, uniqueness, searching.

When a vector is used to store any type of data, it is required to declare a purpose-specific vector type like:

ZBX_VECTOR_DECL(lld_rule_map, zbx_lld_rule_map_t)
       ZBX_VECTOR_IMPL(lld_rule_map, zbx_lld_rule_map_t)

: Where zbx_lld_rule_map_t is the target structure, lld_rule_map is the valuable part of the target structure name that is used to create the vector type name. '_ptr' postfix in the vector name must be used when the vector contains pointers to structures:

ZBX_PTR_VECTOR_DECL(lld_rule_map_ptr, zbx_lld_rule_map_t *)
       ZBX_PTR_VECTOR_IMPL(lld_rule_map_ptr, zbx_lld_rule_map_t *)

For destroying structure-specific type of data and vector element:

zbx_vector_lld_rule_map_clear_ext(&lld_rules, free_lld_rule_map);

: Where free_lld_rule_map is a callback function which receives the pointer to vector element with the target type like:

void    free_lld_rule_map(zbx_lld_rule_map_t *rule);

: Note: for destroying only the vector element it is possible to use the default provided callback function:

void    zbx_ptr_free(void *data)

The use of zbx_vector_ptr_t vector type is acceptable in case of subject field or platform-dependent type of data structures only. When one type of structure has different content, depending on purpose-specific subject field or platform (Linux, Solaris, etc)

Macros

In macro definitions, #define should be followed by a space and the macro name should be followed by a tab:

#define ZBX_PROCESS_SERVER  0x01

In macro conditionals, #ifdef should be followed by a space:

#ifdef HAVE_LDAP

In nested macro conditionals, # character should remain the first character on the line, the rest should be separated by tabs:

#ifdef HAVE_LIBCURL
       #   include <curl/curl.h>
       #   ifndef HAVE_FUNCTION_CURL_EASY_ESCAPE
       #       define curl_easy_escape(handle, string, length) curl_escape(string, length)
       #   endif
       #endif

However, if there is code inside the conditional the nested macros should not be indented:

#ifndef HAVE_SQLITE
       
       static void inc(int *counter)
       {
           *counter++;
       }
       
       #define INC()   inc(&counter)
       #endif

Conditional compilation should not affect indentation of regular code:

#ifndef _WINDOWS
       int get_cpustat(AGENT_RESULT *result, int cpu_num, int state, int mode);
       #endif

Comments in macro conditionals (e.g. after #else or #endif) are only encouraged if it is not clear what the conditional refers to otherwise:

#ifdef _WINDOWS
       #   include "perfmon.h"
       #   include "perfstat.h"
       #endif

#ifdef _WINDOWS
       
       ...long block...
       
       #else   /* not _WINDOWS */
       
       ...long block...
       
       #endif  /* _WINDOWS */

Using #ifdef or #ifndef for conditional statements are preferred over #if defined() and #if !defined(). However if there is a need for a more complex conditional blocks (e. g. "else if" conditions) using #if and #elif statements is allowed:

#ifdef _WINDOWS
           if (WSAEAFNOSUPPORT == zbx_sock_last_error())
       #else
           if (EAFNOSUPPORT == zbx_sock_last_error())
       #endif

#if defined(HAVE_IBM_DB2)
               int             i;
               SQLRETURN       ret = SQL_SUCCESS;
       #elif defined(HAVE_ORACLE)
               sword           err = OCI_SUCCESS;
               ub4             counter;
       #elif defined(HAVE_POSTGRESQL)
               char            *error = NULL;
       #elif defined(HAVE_SQLITE3)
               int             ret = FAIL;
               char            *error = NULL;
       #endif

It is highly preferred that a macro never ends with a semicolon so that when using it a semicolon is always appended:

#define zbx_stat(path, buf) stat(path, buf)
       ...
       zbx_stat(path, buf);</source>
       Using macros without semicolon is allowed only if not possible otherwise:
       <source lang=c>#define DBPATCH_START()                                 zbx_dbpatch_t   patches[] = {
       #define DBPATCH_ADD(version, duplicates, mandatory)     {DBpatch_##version, version, duplicates, mandatory},
       #define DBPATCH_END()                                   {NULL}};
       ...
       DBPATCH_START()
       
       DBPATCH_ADD(2010001, 0, 1)
       DBPATCH_ADD(2010002, 0, 1)
       ...
       
       DBPATCH_END()

In case of a macro with multiple operations each of them should be placed on a separate line. In order to ensure the macro integration in the code use do {...} while (0) loop:

#define zbx_free(ptr)           \
                                       \
       do                              \
       {                               \
               if (ptr)                \
               {                       \
                       free(ptr);      \
                       ptr = NULL;     \
               }                       \
       }                               \
       while (0)

Preprocessor directives should not be used in macro arguments. For example, the following must be avoided:

result = DBselect(
               "select distinct h.hostid,h.host,o.type,o.scriptid,o.execute_on,o.port"
                   ",o.authtype,o.username,o.password,o.publickey,o.privatekey,o.command"
       #ifdef HAVE_OPENIPMI
                   ",h.ipmi_authtype,h.ipmi_privilege,h.ipmi_username,h.ipmi_password"
       #endif
               " from opcommand o,opcommand_grp og,hosts_groups hg,hosts h"
               " where o.operationid=og.operationid"
                   " and og.groupid=hg.groupid"
                   " and hg.hostid=h.hostid"
                   " and o.operationid=" ZBX_FS_UI64
                   " and h.status=%d"
               " union "
               ...

Here, DBselect() is a macro. The behavior of this code is undefined.

For the purpose of type definition typedef existing_type new_type; should be preferred over #define new_type existing_type;
if macro is needed in a single function, it must be both defined and undefined in that function;
if macro is needed in a group of functions, it must be defined before the group of the functions, undefined right after the group of the functions;
if macro is needed in the whole *.c file, it must be defined at the top of the file and should NOT be undefined.

Compiler-specific issues

This section describes why we do not use full features of our programming language and current best practices because of incompatibility with older compilers and software.

Do not use %zu directive for outputting variables of type size_t. Use macro ZBX_FS_SIZE_T as the format specifier and macro zbx_fs_size_t as the type to convert to in calls to printf():

zabbix_log(LOG_LEVEL_DEBUG, "In %s() datalen:" ZBX_FS_SIZE_T, __function_name, (zbx_fs_size_t)j→buffer_size);

Older versions of glibc do not support z modifier:

zabbix_log(LOG_LEVEL_DEBUG, "In %s() datalen:%zu", __function_name, j→buffer_size);

When writing preprocessor directives, leave the # character is the first column.

#ifdef HAVE_LIBCURL
       #   include <curl/curl.h>
       #   ifndef HAVE_FUNCTION_CURL_EASY_ESCAPE
       #       define curl_easy_escape(handle, string, length) curl_escape(string, length)
       #   endif
       #endif

Some compilers do not recognize the following code as valid:

#ifdef HAVE_LIBCURL
           #include <curl/curl.h>
           #ifndef HAVE_FUNCTION_CURL_EASY_ESCAPE
               #define curl_easy_escape(handle, string, length) curl_escape(string, length)
           #endif
       #endif

When writing conditionally compiled code, avoid splitting if conditional into several parts:

#ifdef _WINDOWS
                   if (PF_INET6 == current_ai→ai_family &&
                       ZBX_TCP_ERROR == setsockopt(s→sockets[s→num_socks], IPPROTO_IPV6, IPV6_V6ONLY, (void *)&on, sizeof(on)))
       #else
                   if (PF_INET6 == current_ai→ai_family &&
                       ZBX_TCP_ERROR == setsockopt(s→sockets[s→num_socks], SOL_IPV6, IPV6_V6ONLY, (void *)&on, sizeof(on)))
       #endif

Some compilers do not recognize the following code as valid:

            if (PF_INET6 == current_ai→ai_family &&
       #ifdef _WINDOWS
                       ZBX_TCP_ERROR == setsockopt(s→sockets[s→num_socks], IPPROTO_IPV6, IPV6_V6ONLY, (void *)&on, sizeof(on)))
       #else
                       ZBX_TCP_ERROR == setsockopt(s→sockets[s→num_socks], SOL_IPV6, IPV6_V6ONLY, (void *)&on, sizeof(on)))
       #endif

Avoid using complicated expressions (more complicated than simple assignment of a single value) in variable declarations. For instance, the following code is good:

char    *p_dst = NULL;

Some compilers have trouble compiling more complicated expressions like the following:

size_t  src_size = strlen(src);
       int feeds = src_size / maxline - (0 != src_size % maxline ? 0 : 1);

C99 features are not allowed in Zabbix agent code. For Zabbix server and proxy, C99 features should be avoided where possible. If the code benefits from better readability then designated initializers and compound literals can be used in Zabbix server and proxy code. Multiline compound literals must use braces indented by one tab:

token = (zbx_token_func_macro_t)
           {
               .macro      = { macro_pos, macro_pos_end },
               .func       = { func_pos, func_pos_end },
               .func_param = { func_param_pos, func_param_pos_end }
           };

Best practices

It is advisable to write arithmetic expressions with as few floating-point operations as possible in order not to accumulate floating-point errors (e.g., ZBX-5717). For instance, the following

value_dbl = (100 * (host→cnt - host→rcv)) / (double)host→cnt;

should be preferred over

value_dbl = 100 * (1 - (double)hosts[h].rcv / (double)hosts[h].cnt);

Makefile.am

It is advisable to do linking a static module depend on configure parameter keys. For instance, the following

if HAVE_IPMI
       zabbix_server_LDADD += ipmi/libipmi.a
       endif

When using THIS_SHOULD_NEVER_HAPPEN additional logging must be used to mention what led to this, to make later investigations easier. For example, we expected an itemid in vector of itemids and it was not found. Then, before THIS_SHOULD_NEVER_HAPPEN we must log reasonable details for investigation, e.g. itemid and vector of itemids. If THIS_SHOULD_NEVER_HAPPEN is placed before exit() command, then LOG_LEVEL_INFORMATION level must be used, otherwise LOG_LEVEL_WARNING.

Includes

.h file that declares .c file function prototypes must be included first in .c file before other includes. That is necessary to make .h files independent and not rely on .c file includes

For example, if there are files X.h and X.c and Y.h:

X.h:

#include "Y.h" <- must include all its dependencies (and not rely on .c file)
       int (Y y);

X.c:

#include "X.h" <- must be first
       #include "zbxnix.h"
       #include "zbxalgo.h"
       and other includes...

Documentation