check and warn about capabilities misconfiguration
[collectd.git] / src / chrony.c
index 2bfb806..0485036 100644 (file)
@@ -2,7 +2,7 @@
  **********************************************************************
  * Copyright (C) Claudius M Zingerli, ZSeng, 2015-2016 
  *
- * Internas roughly based on the ntpd plugin
+ * Internals roughly based on the ntpd plugin
  * Some functions copied from chronyd/web (as marked)
  * 
  * This program is free software; you can redistribute it and/or modify
@@ -18,9 +18,6 @@
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
  * 
- **********************************************************************
-*/
-/* Formated using 'GNU indent -bli0 -ce -ppi 2' and some manual alignment
  * TODO:
  *     - More robust udp parsing (using offsets instead of structs?)
  *       -> Currently chrony parses its data the same way as we do (using structs)
  *       -> Done at higher levels
  */
 
-#include "config.h"
+#include "collectd.h"
+
+#include "common.h"             /* auxiliary functions */
+#include "plugin.h"             /* plugin_register_*, plugin_dispatch_values */
 
-#if HAVE_SYS_TYPES_H
-#  include <sys/types.h>        /* getaddrinfo */
-#endif
-#if HAVE_SYS_SOCKET_H
-#  include <sys/socket.h>
-#endif
 #if HAVE_NETDB_H
-#  include <netdb.h>
+#  include <netdb.h>            /* struct addrinfo */
 #endif
 #if HAVE_ARPA_INET_H
 #  include <arpa/inet.h>        /* ntohs/ntohl */
 #endif
 
-#include "collectd.h"
-#include "common.h"             /* auxiliary functions */
-#include "plugin.h"             /* plugin_register_*, plugin_dispatch_values */
 
 #define CONFIG_KEY_HOST    "Host"
 #define CONFIG_KEY_PORT    "Port"
@@ -60,15 +51,15 @@ static const char *g_config_keys[] = {
   CONFIG_KEY_TIMEOUT
 };
 
-static int g_config_keys_num = STATIC_ARRAY_SIZE(g_config_keys);
-static int g_is_connected = 0;
-static int g_chrony_socket = -1;
-static time_t g_chrony_timeout = -1;
-static char *g_chrony_host = NULL;
-static char *g_chrony_port = NULL;
-static char *g_plugin_instance = NULL;
-static uint32_t g_chrony_rand = 1;
-static uint32_t g_chrony_seq_is_initialized = 0;
+static int       g_config_keys_num = STATIC_ARRAY_SIZE(g_config_keys);
+static int       g_chrony_is_connected;
+static int       g_chrony_socket  = -1;
+static time_t    g_chrony_timeout = -1;
+static char     *g_chrony_plugin_instance;
+static char     *g_chrony_host;
+static char     *g_chrony_port;
+static uint32_t  g_chrony_rand    = 1;
+static uint32_t  g_chrony_seq_is_initialized;
 
 #define PLUGIN_NAME_SHORT "chrony"
 #define PLUGIN_NAME       PLUGIN_NAME_SHORT " plugin"
@@ -114,7 +105,15 @@ typedef enum
   RPY_RTC              = 7
 } eDaemonReplies;
 
-#define ATTRIB_PACKED __attribute__((packed))
+
+#if defined(__GNUC__) || defined (__SUNPRO_C) || defined(lint)
+#  /* extension to enforce struct packing. */
+#  define ATTRIB_PACKED __attribute__((packed))
+#else
+#  error Not defining packed attribute (unknown compiler)
+#  define ATTRIB_PACKED 
+#endif
+
 typedef struct ATTRIB_PACKED
 {
   int32_t value;
@@ -222,7 +221,7 @@ typedef struct ATTRIB_PACKED
   int16_t f_poll;               /* 2^f_poll = Time between polls (s) */
   uint16_t f_stratum;           /* Remote clock stratum */
   uint16_t f_state;             /* 0 = RPY_SD_ST_SYNC,    1 = RPY_SD_ST_UNREACH,   2 = RPY_SD_ST_FALSETICKER */
-  /* 3 = RPY_SD_ST_JITTERY, 4 = RPY_SD_ST_CANDIDATE, 5 = RPY_SD_ST_OUTLIER     */
+                                /* 3 = RPY_SD_ST_JITTERY, 4 = RPY_SD_ST_CANDIDATE, 5 = RPY_SD_ST_OUTLIER     */
   uint16_t f_mode;              /* 0 = RPY_SD_MD_CLIENT,  1 = RPY_SD_MD_PEER,      2 = RPY_SD_MD_REF         */
   uint16_t f_flags;             /* unused */
   uint16_t f_reachability;      /* Bit mask of successfull tries to reach the source */
@@ -308,15 +307,15 @@ static int
 connect_client(const char *p_hostname,
                const char *p_service, int p_family, int p_socktype)
 {
-  struct addrinfo hints, *res = NULL, *ressave = NULL;
+  struct addrinfo *res, *ressave;
   int n, sockfd;
 
-  memset(&hints, 0, sizeof(struct addrinfo));
-
-  hints.ai_family = p_family;
-  hints.ai_socktype = p_socktype;
+  struct addrinfo ai_hints = {
+    .ai_family = p_family,
+    .ai_socktype = p_socktype
+  };
 
-  n = getaddrinfo(p_hostname, p_service, &hints, &res);
+  n = getaddrinfo(p_hostname, p_service, &ai_hints, &res);
 
   if (n < 0)
   {
@@ -358,7 +357,6 @@ niptoha(const tChrony_IPAddr * addr, char *p_buf, size_t p_buf_size)
 {
   int rc = 1;
   unsigned long a, b, c, d, ip;
-  const uint8_t *ip6;
 
   switch (ntohs(addr->f_family))
   {
@@ -374,28 +372,15 @@ niptoha(const tChrony_IPAddr * addr, char *p_buf, size_t p_buf_size)
     rc = snprintf(p_buf, p_buf_size, "%ld.%ld.%ld.%ld", a, b, c, d);
     break;
   case IPADDR_INET6:
-    ip6 = addr->addr.ip6;
-
-#ifdef FEAT_IPV6
-    rc = inet_ntop(AF_INET6, ip6, p_buf, p_bug_size);
-#else
-#  if defined(BYTE_ORDER) && (BYTE_ORDER == BIG_ENDIAN)
-    rc =
-      snprintf(p_buf, p_buf_size,
-               "%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x",
-               ip6[15], ip6[14], ip6[13], ip6[12], ip6[11], ip6[10], ip6[9],
-               ip6[8], ip6[7], ip6[6], ip6[5], ip6[4], ip6[3], ip6[2],
-               ip6[1], ip6[0]);
-#  else
-    rc =
-      snprintf(p_buf, p_buf_size,
-               "%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x:%02x%02x",
-               ip6[0], ip6[1], ip6[2], ip6[3], ip6[4], ip6[5], ip6[6],
-               ip6[7], ip6[8], ip6[9], ip6[10], ip6[11], ip6[12], ip6[13],
-               ip6[14], ip6[15]);
-#  endif
-#endif
+  {
+    const char *rp = inet_ntop(AF_INET6, addr->addr.ip6, p_buf, p_buf_size);
+    if (rp == NULL)
+    {
+      ERROR(PLUGIN_NAME ": Error converting ipv6 address to string. Errno = %d", errno);
+      rc = snprintf(p_buf, p_buf_size, "[UNKNOWN]");
+    }
     break;
+  }
   default:
     rc = snprintf(p_buf, p_buf_size, "[UNKNOWN]");
   }
@@ -405,19 +390,18 @@ niptoha(const tChrony_IPAddr * addr, char *p_buf, size_t p_buf_size)
 
 
 static int
-chrony_set_timeout()
+chrony_set_timeout(void)
 {
-  /*Set the socket's  timeout to g_chrony_timeout; a value of 0 signals infinite timeout */
-  /*Returns 0 on success, !0 on error (check errno) */
+  /* Set the socket's  timeout to g_chrony_timeout; a value of 0 signals infinite timeout */
+  /* Returns 0 on success, !0 on error (check errno) */
 
   struct timeval tv;
   tv.tv_sec = g_chrony_timeout;
   tv.tv_usec = 0;
 
   assert(g_chrony_socket >= 0);
-  if (setsockopt
-      (g_chrony_socket, SOL_SOCKET, SO_RCVTIMEO, (char *) &tv,
-       sizeof(struct timeval)) < 0)
+  if (setsockopt(g_chrony_socket, SOL_SOCKET,
+      SO_RCVTIMEO, (char *) &tv, sizeof(struct timeval)) < 0)
   {
     return CHRONY_RC_FAIL;
   }
@@ -426,21 +410,29 @@ chrony_set_timeout()
 
 
 static int
-chrony_connect()
+chrony_connect(void)
 {
-  /*Connects to the chrony daemon */
-  /*Returns 0 on success, !0 on error (check errno) */
+  /* Connects to the chrony daemon */
+  /* Returns 0 on success, !0 on error (check errno) */
   int socket;
 
   if (g_chrony_host == NULL)
   {
     g_chrony_host = strdup(CHRONY_DEFAULT_HOST);
-    assert(g_chrony_host);
+    if (g_chrony_host == NULL)
+    {
+      ERROR(PLUGIN_NAME ": Error duplicating chrony host name");
+      return CHRONY_RC_FAIL;
+    }
   }
   if (g_chrony_port == NULL)
   {
     g_chrony_port = strdup(CHRONY_DEFAULT_PORT);
-    assert(g_chrony_port);
+    if (g_chrony_port == NULL)
+    {
+      ERROR(PLUGIN_NAME ": Error duplicating chrony port string");
+      return CHRONY_RC_FAIL;
+    }
   }
   if (g_chrony_timeout < 0)
   {
@@ -449,8 +441,7 @@ chrony_connect()
   }
 
   DEBUG(PLUGIN_NAME ": Connecting to %s:%s", g_chrony_host, g_chrony_port);
-  socket =
-    connect_client(g_chrony_host, g_chrony_port, AF_UNSPEC, SOCK_DGRAM);
+  socket = connect_client(g_chrony_host, g_chrony_port, AF_UNSPEC, SOCK_DGRAM);
   if (socket < 0)
   {
     ERROR(PLUGIN_NAME ": Error connecting to daemon. Errno = %d", errno);
@@ -491,7 +482,8 @@ chrony_recv_response(tChrony_Response * p_resp, size_t p_resp_max_size,
     ERROR(PLUGIN_NAME ": Error receiving packet: %s (%d)", strerror(errno),
           errno);
     return CHRONY_RC_FAIL;
-  } else
+  }
+  else
   {
     *p_resp_size = rc;
     return CHRONY_RC_OK;
@@ -508,12 +500,13 @@ chrony_query(const int p_command, tChrony_Request * p_req,
   assert(p_resp);
   assert(p_resp_size);
 
-  if (g_is_connected == 0)
+  if (g_chrony_is_connected == 0)
   {
-    if (chrony_connect() == 0)
+    if (chrony_connect() == CHRONY_RC_OK)
     {
-      g_is_connected = 1;
-    } else
+      g_chrony_is_connected = 1;
+    }
+    else
     {
       ERROR(PLUGIN_NAME ": Unable to connect. Errno = %d", errno);
       return CHRONY_RC_FAIL;
@@ -523,31 +516,31 @@ chrony_query(const int p_command, tChrony_Request * p_req,
   do
   {
     int valid_command = 0;
-    size_t req_size = sizeof(p_req->header) + sizeof(p_req->padding);
+    size_t req_size  = sizeof(p_req->header) + sizeof(p_req->padding);
     size_t resp_size = sizeof(p_resp->header);
     uint16_t resp_code = RPY_NULL;
     switch (p_command)
     {
     case REQ_TRACKING:
-      req_size += sizeof(p_req->body.tracking);
+      req_size  += sizeof(p_req->body.tracking);
       resp_size += sizeof(p_resp->body.tracking);
       resp_code = RPY_TRACKING;
       valid_command = 1;
       break;
     case REQ_N_SOURCES:
-      req_size += sizeof(p_req->body.n_sources);
+      req_size  += sizeof(p_req->body.n_sources);
       resp_size += sizeof(p_resp->body.n_sources);
       resp_code = RPY_N_SOURCES;
       valid_command = 1;
       break;
     case REQ_SOURCE_DATA:
-      req_size += sizeof(p_req->body.source_data);
+      req_size  += sizeof(p_req->body.source_data);
       resp_size += sizeof(p_resp->body.source_data);
       resp_code = RPY_SOURCE_DATA;
       valid_command = 1;
       break;
     case REQ_SOURCE_STATS:
-      req_size += sizeof(p_req->body.source_stats);
+      req_size  += sizeof(p_req->body.source_stats);
       resp_size += sizeof(p_resp->body.source_stats);
       resp_code = RPY_SOURCE_STATS;
       valid_command = 1;
@@ -558,9 +551,7 @@ chrony_query(const int p_command, tChrony_Request * p_req,
     }
 
     if (valid_command == 0)
-    {
       break;
-    }
 
     uint32_t seq_nr = rand_r(&g_chrony_rand);
     p_req->header.f_cmd = htons(p_command);
@@ -570,15 +561,12 @@ chrony_query(const int p_command, tChrony_Request * p_req,
     DEBUG(PLUGIN_NAME ": Sending request (.cmd = %d, .seq = %d)", p_command,
           seq_nr);
     if (chrony_send_request(p_req, req_size) != 0)
-    {
       break;
-    }
 
     DEBUG(PLUGIN_NAME ": Waiting for response");
     if (chrony_recv_response(p_resp, resp_size, p_resp_size) != 0)
-    {
       break;
-    }
+
     DEBUG(PLUGIN_NAME
           ": Received response: .version = %u, .type = %u, .cmd = %u, .reply = %u, .status = %u, .seq = %u",
           p_resp->header.f_version, p_resp->header.f_type,
@@ -675,22 +663,19 @@ ntohf(tFloat p_float)
   uval = ntohl(p_float.value);
   exp = (uval >> FLOAT_COEF_BITS) - FLOAT_COEF_BITS;
   if (exp >= 1 << (FLOAT_EXP_BITS - 1))
-  {
     exp -= 1 << FLOAT_EXP_BITS;
-  }
 
   /* coef = (x << FLOAT_EXP_BITS) >> FLOAT_EXP_BITS; */
   coef = uval % (1U << FLOAT_COEF_BITS);
   if (coef >= 1 << (FLOAT_COEF_BITS - 1))
-  {
     coef -= 1 << FLOAT_COEF_BITS;
-  }
+
   return coef * pow(2.0, exp);
 }
 
 
 static void
-chrony_push_data(char *p_type, char *p_type_inst, double p_value)
+chrony_push_data(const char *p_type, const char *p_type_inst, double p_value)
 {
   value_t values[1];
   value_list_t vl = VALUE_LIST_INIT;
@@ -705,45 +690,43 @@ chrony_push_data(char *p_type, char *p_type_inst, double p_value)
   /* defined as: char hostname_g[DATA_MAX_NAME_LEN]; (never NULL) */
   sstrncpy(vl.host, hostname_g, sizeof(vl.host));
   sstrncpy(vl.plugin, PLUGIN_NAME_SHORT, sizeof(vl.plugin));
-  if (g_plugin_instance != NULL)
+  if (g_chrony_plugin_instance != NULL)
   {
-    sstrncpy(vl.plugin_instance, g_plugin_instance,
+    sstrncpy(vl.plugin_instance, g_chrony_plugin_instance,
              sizeof(vl.plugin_instance));
   }
   if (p_type != NULL)
-  {
     sstrncpy(vl.type, p_type, sizeof(vl.type));
-  }
+
   if (p_type_inst != NULL)
-  {
     sstrncpy(vl.type_instance, p_type_inst, sizeof(vl.type_instance));
-  }
 
   plugin_dispatch_values(&vl);
 }
 
 
 static void
-chrony_push_data_valid(char *p_type, char *p_type_inst, const int p_is_valid,
+chrony_push_data_valid(const char *p_type, const char *p_type_inst, const int p_is_valid,
                        double p_value)
 {
   /* Push real value if p_is_valid is true, push NAN if p_is_valid is not true (idea from ntp plugin) */
   if (p_is_valid == 0)
-  {
     p_value = NAN;
-  }
+
   chrony_push_data(p_type, p_type_inst, p_value);
 }
 
 
 static int
-chrony_init_seq()
+chrony_init_seq(void)
 {
   /* Initialize the sequence number generator from /dev/urandom */
   /* Fallbacks: /dev/random and time(NULL) */
 
+  int fh;
+
   /* Try urandom */
-  int fh = open(URAND_DEVICE_PATH, O_RDONLY);
+  fh = open(URAND_DEVICE_PATH, O_RDONLY);
   if (fh >= 0)
   {
     ssize_t rc = read(fh, &g_chrony_rand, sizeof(g_chrony_rand));
@@ -756,12 +739,13 @@ chrony_init_seq()
     }
     close(fh);
     DEBUG(PLUGIN_NAME ": Seeding RNG from " URAND_DEVICE_PATH);
-  } else
+  }
+  else
   {
     if (errno == ENOENT)
     {
       /* URAND_DEVICE_PATH device not found. Try RAND_DEVICE_PATH as fall-back */
-      int fh = open(RAND_DEVICE_PATH, O_RDONLY);
+      fh = open(RAND_DEVICE_PATH, O_RDONLY);
       if (fh >= 0)
       {
         ssize_t rc = read(fh, &g_chrony_rand, sizeof(g_chrony_rand));
@@ -775,13 +759,15 @@ chrony_init_seq()
         }
         close(fh);
         DEBUG(PLUGIN_NAME ": Seeding RNG from " RAND_DEVICE_PATH);
-      } else
+      }
+      else
       {
         /* Error opening RAND_DEVICE_PATH. Try time(NULL) as fall-back */
         DEBUG(PLUGIN_NAME ": Seeding RNG from time(NULL)");
         g_chrony_rand = time(NULL) ^ getpid();
       }
-    } else
+    }
+    else
     {
       ERROR(PLUGIN_NAME ": Opening random source \'%s\' failed: %s (%d)",
             URAND_DEVICE_PATH, strerror(errno), errno);
@@ -801,47 +787,54 @@ chrony_config(const char *p_key, const char *p_value)
 {
   assert(p_key);
   assert(p_value);
+
   /* Parse config variables */
   if (strcasecmp(p_key, CONFIG_KEY_HOST) == 0)
   {
     if (g_chrony_host != NULL)
-    {
       free(g_chrony_host);
-    }
+
     if ((g_chrony_host = strdup(p_value)) == NULL)
     {
       ERROR(PLUGIN_NAME ": Error duplicating host name");
       return CHRONY_RC_FAIL;
     }
-  } else if (strcasecmp(p_key, CONFIG_KEY_PORT) == 0)
+  }
+  else
   {
-    if (g_chrony_port != NULL)
+    if (strcasecmp(p_key, CONFIG_KEY_PORT) == 0)
     {
-      free(g_chrony_port);
+      if (g_chrony_port != NULL)
+        free(g_chrony_port);
+
+      if ((g_chrony_port = strdup(p_value)) == NULL)
+      {
+        ERROR(PLUGIN_NAME ": Error duplicating port name");
+        return CHRONY_RC_FAIL;
+      }
     }
-    if ((g_chrony_port = strdup(p_value)) == NULL)
+    else
     {
-      ERROR(PLUGIN_NAME ": Error duplicating port name");
-      return CHRONY_RC_FAIL;
+      if (strcasecmp(p_key, CONFIG_KEY_TIMEOUT) == 0)
+      {
+        time_t tosec = strtol(p_value, NULL, 0);
+        g_chrony_timeout = tosec;
+      }
+      else
+      {
+        WARNING(PLUGIN_NAME ": Unknown configuration variable: %s %s", p_key, p_value);
+        return CHRONY_RC_FAIL;
+      }
     }
-  } else if (strcasecmp(p_key, CONFIG_KEY_TIMEOUT) == 0)
-  {
-    time_t tosec = strtol(p_value, NULL, 0);
-    g_chrony_timeout = tosec;
-  } else
-  {
-    WARNING(PLUGIN_NAME ": Unknown configuration variable: %s %s", p_key,
-            p_value);
-    return CHRONY_RC_FAIL;
   }
-  /* XXX: We could set g_plugin_instance here to "g_chrony_host-g_chrony_port", but as multiple instances aren't yet supported, we skip this for now */
+  /* XXX: We could set g_chrony_plugin_instance here to "g_chrony_host-g_chrony_port", but as multiple instances aren't yet supported, we skip this for now */
 
   return CHRONY_RC_OK;
 }
 
 
 static int
-chrony_request_daemon_stats()
+chrony_request_daemon_stats(void)
 {
   /* Perform Tracking request */
   int rc;
@@ -860,8 +853,7 @@ chrony_request_daemon_stats()
   }
 #if COLLECT_DEBUG
   {
-    char src_addr[IPV6_STR_MAX_SIZE];
-    memset(src_addr, 0, sizeof(src_addr));
+    char src_addr[IPV6_STR_MAX_SIZE] = { 0 };
     niptoha(&chrony_resp.body.tracking.addr, src_addr, sizeof(src_addr));
     DEBUG(PLUGIN_NAME ": Daemon stat: .addr = %s, .ref_id= %u, .stratum = %u, .leap_status = %u, .ref_time = %u:%u:%u, .current_correction = %f, .last_offset = %f, .rms_offset = %f, .freq_ppm = %f, .skew_ppm = %f, .root_delay = %f, .root_dispersion = %f, .last_update_interval = %f", src_addr, ntohs(chrony_resp.body.tracking.f_ref_id),  
           ntohs(chrony_resp.body.tracking.f_stratum),
@@ -898,10 +890,10 @@ chrony_request_daemon_stats()
   chrony_push_data("time_ref",          DAEMON_NAME, time_ref);  /* unit: s */
   chrony_push_data("time_offset_ntp",   DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_current_correction));      /* Offset between system time and NTP, unit: s */
   chrony_push_data("time_offset",       DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_last_offset)); /* Estimated Offset of the NTP time, unit: s */
-  chrony_push_data("time_offset_rms",   DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_rms_offset));      /* averaged value of the above, unit: s */
-  chrony_push_data("frequency_error",   DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_freq_ppm));        /* Frequency error of the local osc, unit: ppm */
+  chrony_push_data("time_offset_rms",   DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_rms_offset));  /* averaged value of the above, unit: s */
+  chrony_push_data("frequency_error",   DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_freq_ppm));    /* Frequency error of the local osc, unit: ppm */
   chrony_push_data("clock_skew_ppm",    DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_skew_ppm));
-  chrony_push_data("root_delay",        DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_root_delay));   /* Network latency between local daemon and the current source */
+  chrony_push_data("root_delay",        DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_root_delay));  /* Network latency between local daemon and the current source */
   chrony_push_data("root_dispersion",   DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_root_dispersion));
   chrony_push_data("clock_last_update", DAEMON_NAME, ntohf(chrony_resp.body.tracking.f_last_update_interval));
 
@@ -945,8 +937,7 @@ chrony_request_source_data(int p_src_idx, int *p_is_reachable)
   tChrony_Request chrony_req;
   tChrony_Response chrony_resp;
 
-  char src_addr[IPV6_STR_MAX_SIZE];
-  memset(src_addr, 0, sizeof(src_addr));
+  char src_addr[IPV6_STR_MAX_SIZE] = { 0 };
 
   chrony_init_req(&chrony_req);
   chrony_req.body.source_data.f_index = htonl(p_src_idx);
@@ -1005,15 +996,15 @@ chrony_request_source_stats(int p_src_idx, const int *p_is_reachable)
   tChrony_Response chrony_resp;
   double skew_ppm, frequency_error, time_offset;
 
-  char src_addr[IPV6_STR_MAX_SIZE];
-  memset(src_addr, 0, sizeof(src_addr));
+  char src_addr[IPV6_STR_MAX_SIZE] = { 0 };
 
   if (*p_is_reachable == 0)
   {
     skew_ppm = 0;
     frequency_error = 0;
     time_offset = 0;
-  } else
+  }
+  else
   {
     chrony_init_req(&chrony_req);
     chrony_req.body.source_stats.f_index = htonl(p_src_idx);
@@ -1046,93 +1037,78 @@ chrony_request_source_stats(int p_src_idx, const int *p_is_reachable)
           frequency_error, skew_ppm, time_offset,
           ntohf(chrony_resp.body.source_stats.f_est_offset_err));
 
-  }                             /* if (*is_reachable) */
+  } /* if (*is_reachable) */
 
   /* Forward results to collectd-daemon */
-  chrony_push_data_valid("clock_skew_ppm", src_addr, *p_is_reachable,
-                         skew_ppm);
-  chrony_push_data_valid("frequency_error", src_addr, *p_is_reachable, frequency_error);        /* unit: ppm */
-  chrony_push_data_valid("time_offset", src_addr, *p_is_reachable, time_offset);        /* unit: s */
+  chrony_push_data_valid("clock_skew_ppm", src_addr, *p_is_reachable, skew_ppm);
+  chrony_push_data_valid("frequency_error", src_addr, *p_is_reachable, frequency_error); /* unit: ppm */
+  chrony_push_data_valid("time_offset", src_addr, *p_is_reachable, time_offset);         /* unit: s */
 
   return CHRONY_RC_OK;
 }
 
 
 static int
-chrony_read()
+chrony_read(void)
 {
   /* collectd read callback: Perform data acquisition */
   int rc;
-  unsigned int now_src, n_sources;
+  unsigned int n_sources;
 
   if (g_chrony_seq_is_initialized == 0)
   {
     /* Seed RNG for sequence number generation */
     rc = chrony_init_seq();
     if (rc != CHRONY_RC_OK)
-    {
       return rc;
-    }
+
     g_chrony_seq_is_initialized = 1;
   }
 
   /* Get daemon stats */
   rc = chrony_request_daemon_stats();
   if (rc != CHRONY_RC_OK)
-  {
     return rc;
-  }
 
   /* Get number of time sources, then check every source for status */
   rc = chrony_request_sources_count(&n_sources);
   if (rc != CHRONY_RC_OK)
-  {
     return rc;
-  }
 
-  for (now_src = 0; now_src < n_sources; ++now_src)
+  for (unsigned int now_src = 0; now_src < n_sources; ++now_src)
   {
     int is_reachable;
     rc = chrony_request_source_data(now_src, &is_reachable);
     if (rc != CHRONY_RC_OK)
-    {
       return rc;
-    }
 
     rc = chrony_request_source_stats(now_src, &is_reachable);
     if (rc != CHRONY_RC_OK)
-    {
       return rc;
-    }
+
   }
   return CHRONY_RC_OK;
 }
 
 
 static int
-chrony_shutdown()
+chrony_shutdown(void)
 {
   /* Collectd shutdown callback: Free mem */
-  if (g_is_connected != 0)
+  if (g_chrony_is_connected != 0)
   {
     close(g_chrony_socket);
-    g_is_connected = 0;
+    g_chrony_is_connected = 0;
   }
   if (g_chrony_host != NULL)
-  {
-    free(g_chrony_host);
-    g_chrony_host = NULL;
-  }
+    sfree(g_chrony_host);
+
   if (g_chrony_port != NULL)
-  {
-    free(g_chrony_port);
-    g_chrony_port = NULL;
-  }
-  if (g_plugin_instance != NULL)
-  {
-    free(g_plugin_instance);
-    g_plugin_instance = NULL;
-  }
+    sfree(g_chrony_port);
+  
+  if (g_chrony_plugin_instance != NULL)
+    sfree(g_chrony_plugin_instance);
+  
   return CHRONY_RC_OK;
 }