Fixed various signedness issues identified by -Wextra.
authorSebastian Harl <sh@tokkee.org>
Wed, 11 Feb 2009 10:31:30 +0000 (11:31 +0100)
committerSebastian Harl <sh@tokkee.org>
Wed, 11 Feb 2009 10:31:30 +0000 (11:31 +0100)
The following two issues have been addressed:

 * comparison between signed and unsigned - this was found in several places
   throughout the code and has been fixed by switching to more appropriate
   types or adding appropriate explicit casts.
 * comparison of unsigned expression < 0 is always false - this was found in
   the processes and vserver plugins where a size_t had wrongly been used
   instead of a ssize_t and an int respectively.

20 files changed:
src/apache.c
src/ascent.c
src/collectd-nagios.c
src/common.c
src/configfile.c
src/netlink.c
src/network.c
src/nginx.c
src/plugin.c
src/processes.c
src/snmp.c
src/tail.c
src/thermal.c
src/utils_cache.c
src/utils_cmd_getval.c
src/utils_match.c
src/utils_rrdcreate.c
src/utils_subst.c
src/utils_tail_match.c
src/vserver.c

index 091d594..4fa7aa1 100644 (file)
@@ -148,7 +148,7 @@ static int init (void)
 
                status = ssnprintf (credentials, sizeof (credentials), "%s:%s",
                                user, (pass == NULL) ? "" : pass);
-               if (status >= sizeof (credentials))
+               if ((status < 0) || ((size_t) status >= sizeof (credentials)))
                {
                        ERROR ("apache plugin: init: Returning an error "
                                        "because the credentials have been "
index 54341c7..8829e51 100644 (file)
@@ -174,7 +174,7 @@ static size_t ascent_curl_callback (void *buf, size_t size, size_t nmemb, /* {{{
 
 static int ascent_submit_players (player_stats_t *ps) /* {{{ */
 {
-  int i;
+  size_t i;
   gauge_t value;
 
   for (i = 0; i < RACES_LIST_LENGTH; i++)
@@ -213,7 +213,7 @@ static int ascent_account_player (player_stats_t *ps, /* {{{ */
 {
   if (pi->race >= 0)
   {
-    if ((pi->race >= RACES_LIST_LENGTH)
+    if (((size_t) pi->race >= RACES_LIST_LENGTH)
         || (races_list[pi->race] == NULL))
       ERROR ("ascent plugin: Ignoring invalid numeric race %i.", pi->race);
     else
@@ -222,7 +222,7 @@ static int ascent_account_player (player_stats_t *ps, /* {{{ */
 
   if (pi->class >= 0)
   {
-    if ((pi->class >= CLASSES_LIST_LENGTH)
+    if (((size_t) pi->class >= CLASSES_LIST_LENGTH)
         || (classes_list[pi->class] == NULL))
       ERROR ("ascent plugin: Ignoring invalid numeric class %i.", pi->class);
     else
@@ -231,7 +231,7 @@ static int ascent_account_player (player_stats_t *ps, /* {{{ */
 
   if (pi->gender >= 0)
   {
-    if ((pi->gender >= GENDERS_LIST_LENGTH)
+    if (((size_t) pi->gender >= GENDERS_LIST_LENGTH)
         || (genders_list[pi->gender] == NULL))
       ERROR ("ascent plugin: Ignoring invalid numeric gender %i.",
           pi->gender);
@@ -549,7 +549,7 @@ static int ascent_init (void) /* {{{ */
 
     status = ssnprintf (credentials, sizeof (credentials), "%s:%s",
         user, (pass == NULL) ? "" : pass);
-    if (status >= sizeof (credentials))
+    if ((status < 0) || ((size_t) status >= sizeof (credentials)))
     {
       ERROR ("ascent plugin: ascent_init: Returning an error because the "
           "credentials have been truncated.");
index b995b1c..8f4a410 100644 (file)
@@ -165,7 +165,7 @@ static int filter_ds (size_t *values_num,
                return (RET_UNKNOWN);
        }
 
-       for (i = 0; i < match_ds_num_g; i++)
+       for (i = 0; i < (size_t) match_ds_num_g; i++)
        {
                size_t j;
 
@@ -298,7 +298,7 @@ static int do_check_con_none (size_t values_num,
        int num_okay = 0;
        const char *status_str = "UNKNOWN";
        int status_code = RET_UNKNOWN;
-       int i;
+       size_t i;
 
        for (i = 0; i < values_num; i++)
        {
@@ -349,7 +349,7 @@ static int do_check_con_none (size_t values_num,
 static int do_check_con_average (size_t values_num,
                double *values, char **values_names)
 {
-       int i;
+       size_t i;
        double total;
        int total_num;
        double average;
@@ -402,7 +402,7 @@ static int do_check_con_average (size_t values_num,
 static int do_check_con_sum (size_t values_num,
                double *values, char **values_names)
 {
-       int i;
+       size_t i;
        double total;
        int total_num;
        const char *status_str = "UNKNOWN";
@@ -452,7 +452,7 @@ static int do_check_con_sum (size_t values_num,
 static int do_check_con_percentage (size_t values_num,
                double *values, char **values_names)
 {
-       int i;
+       size_t i;
        double sum = 0.0;
        double percentage;
 
index 7cc1d9d..28f16da 100644 (file)
@@ -411,7 +411,7 @@ int check_create_dir (const char *file_orig)
        char *saveptr;
        int   last_is_file = 1;
        int   path_is_absolute = 0;
-       int   len;
+       size_t len;
        int   i;
 
        /*
index 0f49de2..bb1770d 100644 (file)
@@ -555,7 +555,7 @@ static oconfig_item_t *cf_read_dir (const char *dir, int depth)
 
                status = ssnprintf (name, sizeof (name), "%s/%s",
                                dir, de->d_name);
-               if (status >= sizeof (name))
+               if ((status < 0) || ((size_t) status >= sizeof (name)))
                {
                        ERROR ("configfile: Not including `%s/%s' because its"
                                        " name is too long.",
@@ -630,7 +630,7 @@ static oconfig_item_t *cf_read_generic (const char *path, int depth)
        int status;
        const char *path_ptr;
        wordexp_t we;
-       int i;
+       size_t i;
 
        if (depth >= CF_MAX_DEPTH)
        {
index 8f45ea2..b15768e 100644 (file)
@@ -246,7 +246,7 @@ static int link_filter (const struct sockaddr_nl __attribute__((unused)) *sa,
 
   /* Update the `iflist'. It's used to know which interfaces exist and query
    * them later for qdiscs and classes. */
-  if (msg->ifi_index >= iflist_len)
+  if ((msg->ifi_index >= 0) && ((size_t) msg->ifi_index >= iflist_len))
   {
     char **temp;
 
@@ -359,7 +359,8 @@ static int qos_filter (const struct sockaddr_nl __attribute__((unused)) *sa,
     return (0);
   }
 
-  if (msg->tcm_ifindex >= iflist_len)
+  if ((msg->tcm_ifindex >= 0)
+      && ((size_t) msg->tcm_ifindex >= iflist_len))
   {
     ERROR ("netlink plugin: qos_filter: msg->tcm_ifindex = %i "
        ">= iflist_len = %zu",
@@ -580,9 +581,9 @@ static int ir_read (void)
 
   /* `link_filter' will update `iflist' which is used here to iterate over all
    * interfaces. */
-  for (ifindex = 0; ifindex < iflist_len; ifindex++)
+  for (ifindex = 0; (size_t) ifindex < iflist_len; ifindex++)
   {
-    int type_index;
+    size_t type_index;
 
     if (iflist[ifindex] == NULL)
       continue;
index e080000..66f0438 100644 (file)
@@ -498,7 +498,7 @@ static int parse_part_values (void **ret_buffer, int *ret_buffer_len,
 
        exp_size = 3 * sizeof (uint16_t)
                + pkg_numval * (sizeof (uint8_t) + sizeof (value_t));
-       if (buffer_len < exp_size)
+       if ((buffer_len < 0) || ((size_t) buffer_len < exp_size))
        {
                WARNING ("network plugin: parse_part_values: "
                                "Packet too short: "
@@ -562,7 +562,7 @@ static int parse_part_number (void **ret_buffer, int *ret_buffer_len,
        uint16_t pkg_length;
        uint16_t pkg_type;
 
-       if (buffer_len < exp_size)
+       if ((buffer_len < 0) || ((size_t) buffer_len < exp_size))
        {
                WARNING ("network plugin: parse_part_number: "
                                "Packet too short: "
@@ -602,7 +602,7 @@ static int parse_part_string (void **ret_buffer, int *ret_buffer_len,
        uint16_t pkg_length;
        uint16_t pkg_type;
 
-       if (buffer_len < header_size)
+       if ((buffer_len < 0) || ((size_t) buffer_len < header_size))
        {
                WARNING ("network plugin: parse_part_string: "
                                "Packet too short: "
@@ -644,7 +644,8 @@ static int parse_part_string (void **ret_buffer, int *ret_buffer_len,
        /* Check that the package data fits into the output buffer.
         * The previous if-statement ensures that:
         * `pkg_length > header_size' */
-       if ((pkg_length - header_size) > output_len)
+       if ((output_len < 0)
+                       || ((size_t) output_len < ((size_t) pkg_length - header_size)))
        {
                WARNING ("network plugin: parse_part_string: "
                                "Output buffer too small.");
index dfa4f98..2de3633 100644 (file)
@@ -123,8 +123,9 @@ static int init (void)
 
   if (user != NULL)
   {
-    if (ssnprintf (credentials, sizeof (credentials),
-         "%s:%s", user, pass == NULL ? "" : pass) >= sizeof (credentials))
+    int status = ssnprintf (credentials, sizeof (credentials),
+       "%s:%s", user, pass == NULL ? "" : pass);
+    if ((status < 0) || ((size_t) status >= sizeof (credentials)))
     {
       ERROR ("nginx plugin: Credentials would have been truncated.");
       return (-1);
index 9d558cb..61cc09c 100644 (file)
@@ -328,6 +328,7 @@ int plugin_load (const char *type)
        int   ret;
        struct stat    statbuf;
        struct dirent *de;
+       int status;
 
        DEBUG ("type = %s", type);
 
@@ -336,8 +337,8 @@ int plugin_load (const char *type)
 
        /* `cpu' should not match `cpufreq'. To solve this we add `.so' to the
         * type when matching the filename */
-       if (ssnprintf (typename, sizeof (typename),
-                       "%s.so", type) >= sizeof (typename))
+       status = ssnprintf (typename, sizeof (typename), "%s.so", type);
+       if ((status < 0) || ((size_t) status >= sizeof (typename)))
        {
                WARNING ("snprintf: truncated: `%s.so'", type);
                return (-1);
@@ -357,8 +358,9 @@ int plugin_load (const char *type)
                if (strncasecmp (de->d_name, typename, typename_len))
                        continue;
 
-               if (ssnprintf (filename, sizeof (filename),
-                               "%s/%s", dir, de->d_name) >= sizeof (filename))
+               status = ssnprintf (filename, sizeof (filename),
+                               "%s/%s", dir, de->d_name);
+               if ((status < 0) || ((size_t) status >= sizeof (filename)))
                {
                        WARNING ("snprintf: truncated: `%s/%s'", dir, de->d_name);
                        continue;
index 553b195..26dde3e 100644 (file)
@@ -821,7 +821,7 @@ static char *ps_get_cmdline (pid_t pid, char *name, char *buf, size_t buf_len)
        n = 0;
 
        while (42) {
-               size_t status;
+               ssize_t status;
 
                status = read (fd, (void *)buf_ptr, len);
 
index 82fd658..998c4b6 100644 (file)
@@ -530,9 +530,9 @@ static int csnmp_config_add_host_interval (host_definition_t *hd, oconfig_item_t
     return (-1);
   }
 
-  hd->interval = (int) ci->values[0].value.number;
-  if (hd->interval < 0)
-    hd->interval = 0;
+  hd->interval = ci->values[0].value.number >= 0
+    ? (uint32_t) ci->values[0].value.number
+    : 0;
 
   return (0);
 } /* int csnmp_config_add_host_interval */
@@ -1138,7 +1138,7 @@ static int csnmp_read_table (host_definition_t *host, data_definition_t *data)
       break;
     }
 
-    for (i = 0; i < oid_list_len; i++)
+    for (i = 0; (uint32_t) i < oid_list_len; i++)
       snmp_add_null_var (req, oid_list[i].oid, oid_list[i].oid_len);
 
     res = NULL;
@@ -1441,7 +1441,7 @@ static int csnmp_read_host (host_definition_t *host)
   time_end = time (NULL);
   DEBUG ("snmp plugin: csnmp_read_host (%s) finished at %u;", host->name,
       (unsigned int) time_end);
-  if ((time_end - time_start) > host->interval)
+  if ((uint32_t) (time_end - time_start) > host->interval)
   {
     WARNING ("snmp plugin: Host `%s' should be queried every %i seconds, "
        "but reading all values takes %u seconds.",
@@ -1504,7 +1504,7 @@ static int csnmp_init (void)
     {
       host->interval = interval_g;
     }
-    else if (host->interval < interval_g)
+    else if (host->interval < (uint32_t) interval_g)
     {
       host->interval = interval_g;
       WARNING ("snmp plugin: Data for host `%s' will be collected every %i seconds.",
index 01bf629..3a98c60 100644 (file)
@@ -305,7 +305,7 @@ static int ctail_init (void)
 static int ctail_read (void)
 {
   int success = 0;
-  int i;
+  size_t i;
 
   for (i = 0; i < tail_match_list_num; i++)
   {
@@ -329,7 +329,7 @@ static int ctail_read (void)
 
 static int ctail_shutdown (void)
 {
-  int i;
+  size_t i;
 
   for (i = 0; i < tail_match_list_num; i++)
   {
index 2ea6a24..fe54aee 100644 (file)
@@ -71,8 +71,9 @@ static int thermal_sysfs_device_read (const char __attribute__((unused)) *dir,
        if (device_list && ignorelist_match (device_list, name))
                return -1;
 
-       len = snprintf (filename, sizeof (filename), "%s/%s/temp", dirname_sysfs, name);
-       if ((len < 0) || ((unsigned int)len >= sizeof (filename)))
+       len = snprintf (filename, sizeof (filename),
+                       "%s/%s/temp", dirname_sysfs, name);
+       if ((len < 0) || ((size_t) len >= sizeof (filename)))
                return -1;
 
        len = read_file_contents (filename, data, sizeof(data));
@@ -90,8 +91,9 @@ static int thermal_sysfs_device_read (const char __attribute__((unused)) *dir,
                }
        }
 
-       len = snprintf (filename, sizeof (filename), "%s/%s/cur_state", dirname_sysfs, name);
-       if ((len < 0) || ((unsigned int)len >= sizeof (filename)))
+       len = snprintf (filename, sizeof (filename),
+                       "%s/%s/cur_state", dirname_sysfs, name);
+       if ((len < 0) || ((size_t) len >= sizeof (filename)))
                return -1;
 
        len = read_file_contents (filename, data, sizeof(data));
@@ -128,12 +130,15 @@ static int thermal_procfs_device_read (const char __attribute__((unused)) *dir,
         * temperature:             55 C
         */
        
-       len = snprintf (filename, sizeof (filename), "%s/%s/temperature", dirname_procfs, name);
-       if ((len < 0) || ((unsigned int)len >= sizeof (filename)))
+       len = snprintf (filename, sizeof (filename),
+                       "%s/%s/temperature", dirname_procfs, name);
+       if ((len < 0) || ((size_t) len >= sizeof (filename)))
                return -1;
 
        len = read_file_contents (filename, data, sizeof(data));
-       if (len > sizeof(str_temp) && data[--len] == '\n' && !strncmp(data, str_temp, sizeof(str_temp)-1)) {
+       if ((len > 0) && ((size_t) len > sizeof(str_temp))
+                       && (data[--len] == '\n')
+                       && (! strncmp(data, str_temp, sizeof(str_temp)-1))) {
                char *endptr = NULL;
                double temp;
                double celsius, add;
index 824b7c8..b2f2c36 100644 (file)
@@ -522,7 +522,7 @@ gauge_t *uc_get_rate (const data_set_t *ds, const value_list_t *vl)
 
   /* This is important - the caller has no other way of knowing how many
    * values are returned. */
-  if (ret_num != ds->ds_num)
+  if (ret_num != (size_t) ds->ds_num)
   {
     ERROR ("utils_cache: uc_get_rate: ds[%s] has %i values, "
        "but uc_get_rate_by_name returned %zu.",
index 186ef9b..ce3e28e 100644 (file)
@@ -51,7 +51,7 @@ int handle_getval (FILE *fh, char *buffer)
   const data_set_t *ds;
 
   int   status;
-  int   i;
+  size_t i;
 
   if ((fh == NULL) || (buffer == NULL))
     return (-1);
@@ -123,7 +123,7 @@ int handle_getval (FILE *fh, char *buffer)
     return (-1);
   }
 
-  if (ds->ds_num != values_num)
+  if ((size_t) ds->ds_num != values_num)
   {
     ERROR ("ds[%s]->ds_num = %i, "
        "but uc_get_rate_by_name returned %u values.",
index 906d7c7..c19c5ff 100644 (file)
@@ -50,7 +50,7 @@ static char *match_substr (const char *str, int begin, int end)
 
   if ((begin < 0) || (end < 0) || (begin >= end))
     return (NULL);
-  if (end > (strlen (str) + 1))
+  if ((size_t) end > (strlen (str) + 1))
   {
     ERROR ("utils_match: match_substr: `end' points after end of string.");
     return (NULL);
@@ -228,7 +228,7 @@ int match_apply (cu_match_t *obj, const char *str)
   regmatch_t re_match[32];
   char *matches[32];
   size_t matches_num;
-  int i;
+  size_t i;
 
   if ((obj == NULL) || (str == NULL))
     return (-1);
index f006d13..c4e9d8b 100644 (file)
@@ -148,12 +148,15 @@ static int rra_get (char ***ret, const value_list_t *vl, /* {{{ */
 
     for (j = 0; j < rra_types_num; j++)
     {
+      int status;
+
       if (rra_num >= rra_max)
         break;
 
-      if (ssnprintf (buffer, sizeof (buffer), "RRA:%s:%3.1f:%u:%u",
-            rra_types[j], cfg->xff,
-            cdp_len, cdp_num) >= sizeof (buffer))
+      status = ssnprintf (buffer, sizeof (buffer), "RRA:%s:%3.1f:%u:%u",
+          rra_types[j], cfg->xff, cdp_len, cdp_num);
+
+      if ((status < 0) || ((size_t) status >= sizeof (buffer)))
       {
         ERROR ("rra_get: Buffer would have been truncated.");
         continue;
@@ -236,7 +239,7 @@ static int ds_get (char ***ret, /* {{{ */
         d->name, type,
         (cfg->heartbeat > 0) ? cfg->heartbeat : (2 * vl->interval),
         min, max);
-    if ((status < 1) || (status >= sizeof (buffer)))
+    if ((status < 1) || ((size_t) status >= sizeof (buffer)))
       break;
 
     ds_def[ds_num] = sstrdup (buffer);
index 640007c..a49f6db 100644 (file)
@@ -37,7 +37,8 @@ char *subst (char *buf, size_t buflen, const char *string, int off1, int off2,
                        || (NULL == replacement))
                return NULL;
 
-       sstrncpy (buf_ptr, string, (off1 + 1 > buflen) ? buflen : off1 + 1);
+       sstrncpy (buf_ptr, string,
+                       ((size_t)off1 + 1 > buflen) ? buflen : (size_t)off1 + 1);
        buf_ptr += off1;
        len     -= off1;
 
index ae1f4f7..22c7917 100644 (file)
@@ -103,7 +103,7 @@ static int tail_callback (void *data, char *buf,
     int __attribute__((unused)) buflen)
 {
   cu_tail_match_t *obj = (cu_tail_match_t *) data;
-  int i;
+  size_t i;
 
   for (i = 0; i < obj->matches_num; i++)
     match_apply (obj->matches[i].match, buf);
@@ -135,7 +135,7 @@ cu_tail_match_t *tail_match_create (const char *filename)
 
 void tail_match_destroy (cu_tail_match_t *obj)
 {
-  int i;
+  size_t i;
 
   if (obj == NULL)
     return;
@@ -237,7 +237,7 @@ int tail_match_read (cu_tail_match_t *obj)
 {
   char buffer[4096];
   int status;
-  int i;
+  size_t i;
 
   status = cu_tail_read (obj->tail, buffer, sizeof (buffer), tail_callback,
       (void *) obj);
index 8747d9b..8fdae27 100644 (file)
@@ -139,7 +139,7 @@ static int vserver_read (void)
 
        while (42)
        {
-               size_t len;
+               int len;
                char file[BUFSIZE];
 
                FILE *fh;
@@ -168,7 +168,7 @@ static int vserver_read (void)
                if (dent->d_name[0] == '.')
                        continue;
 
-               len = snprintf (file, sizeof (file), PROCDIR "/%s", dent->d_name);
+               len = ssnprintf (file, sizeof (file), PROCDIR "/%s", dent->d_name);
                if ((len < 0) || (len >= BUFSIZE))
                        continue;
                
@@ -187,7 +187,7 @@ static int vserver_read (void)
                /* socket message accounting */
                len = ssnprintf (file, sizeof (file),
                                PROCDIR "/%s/cacct", dent->d_name);
-               if ((len < 0) || (len >= sizeof (file)))
+               if ((len < 0) || ((size_t) len >= sizeof (file)))
                        continue;
 
                if (NULL == (fh = fopen (file, "r")))
@@ -235,7 +235,7 @@ static int vserver_read (void)
                /* thread information and load */
                len = ssnprintf (file, sizeof (file),
                                PROCDIR "/%s/cvirt", dent->d_name);
-               if ((len < 0) || (len >= sizeof (file)))
+               if ((len < 0) || ((size_t) len >= sizeof (file)))
                        continue;
 
                if (NULL == (fh = fopen (file, "r")))
@@ -288,7 +288,7 @@ static int vserver_read (void)
                /* processes and memory usage */
                len = ssnprintf (file, sizeof (file),
                                PROCDIR "/%s/limit", dent->d_name);
-               if ((len < 0) || (len >= sizeof (file)))
+               if ((len < 0) || ((size_t) len >= sizeof (file)))
                        continue;
 
                if (NULL == (fh = fopen (file, "r")))