struct utsname name; if (-1 == uname(&name)) return SYSINFO_RET_FAIL;
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));
*lastlogsize = (long)buf.st_size;
zabbix_log(LOG_LEVEL_WARNING, "cannot remove shared memory for collector [%s]", strerror(errno));
#define MEM_MAX_SIZE 0x7fffffff
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);
PERFCOUNTER *counterName = NULL; DWORD dwSize;
error = zbx_sock_last_error(); /* zabbix_log() resets the error code */
zabbix_log(LOG_LEVEL_CRIT, "failed assumption about pointer size (%lu not in {4, 8})", ZBX_PTR_SIZE);
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;
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} };
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; }
/* check whether we need to update items_hk index */
/* The agent expects ALWAYS to have lastlogsize and mtime tags. Removing those would cause older agents to fail. */
/* do not reallocate if not much is freed */ if (size > chunk_size / 4) return chunk;
{
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; ... }
free_perf_collector(); /* cpu_collector must be freed before perf_collector is freed */
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; }
/****************************************************************************** * * * 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, ...
if (SUCCEED == str_in_list("last,prev", function, ','))
if (2 < num_param(param))
if (0x80 == (*text & 0xc0))
0
or NULL
explicitly:
if (NULL != message_esc && 0 != message_esc_len)
if ('1' <= *(br - 2) && *(br - 2) <= '9')
is_numeric |= (SUCCEED == _wis_uint(cpe->szCounterName) ? 0x02 : 0);
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
, 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; }
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; }
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); }
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);
The vector implementation supports various operations such as sorting, uniqueness, searching.
ZBX_PTR_VECTOR_DECL(lld_rule_map, zbx_lld_rule_map_t *) ZBX_PTR_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.
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)
#define
should be followed by a space and the macro name should be followed by a tab:
#define ZBX_PROCESS_SERVER 0x01
#ifdef
should be followed by a space:
#ifdef HAVE_LDAP
#
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
#ifndef _WINDOWS int get_cpustat(AGENT_RESULT *result, int cpu_num, int state, int mode); #endif
#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 */
#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
#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()
do {…} while (0)
loop:
#define zbx_free(ptr) \
\
do \
{ \
if (ptr) \
{ \
free(ptr); \
ptr = NULL; \
} \
} \
while (0)
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.
typedef existing_type new_type;
should be preferred over #define new_type existing_type
.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.
%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);
#
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
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
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);
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 } };
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);
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