Code

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 091d594741612e3f12d99f3852af58bf90785fbd..4fa7aa1b11a9f8dbf192271abfe65322be0e3806 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 54341c7733cfa116cfe79d463eb7fec81159f50e..8829e518f3fbfef289b9d74d368fe7ffc1de7bbf 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 b995b1c58efa28af61a912a7bb86b24597788e31..8f4a41054ab55efbf0e79bf4d2a51b3da0e9244e 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 7cc1d9d44e591f5a93a917016752350cf2d2a649..28f16da0190ada6346806335f8f0ba484afc7f72 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 0f49de266f643929632895ca4d76b7630732bb60..bb1770d2ab13e8c8a26c84901dd9e012dd0fc9a7 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 8f45ea2e79334927b3b29942a3f33296bb9f2a6e..b15768e719d5c7ea847848ea63db4142e703c45c 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 e080000442f25f58aa5dda6f8b4c3a0b38d555bd..66f04380388fc3ad20d56d3f95333567e55bebfe 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 dfa4f9832f0252acdfe789d5effc7e560ebcaa4c..2de3633b41c6ffdc2feb1ffc64d329bca7092f9b 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 9d558cb1941f54df06ed2523398c0ae61abd4c62..61cc09c6251500c275ddee80582cd7bcbb8f7bab 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 553b1955c2f868904f728f315203bdbb848327c4..26dde3e4f43e91d29ae4955e66eae705ebdd1409 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 82fd6583b075b5f243d06d6e85b9cb86803302a0..998c4b648fbffde71a2dc9cd5f056b80ff266df4 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 01bf6292ac2d1583796d42d387a69b4080d6752f..3a98c60f0c67436c9ea9804d9bf272a925eda568 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 2ea6a24fb916719937376556f0bc8d3f6d3d7f41..fe54aee443790617b30c22915414bcd4c5123b5c 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 824b7c80bcf6d64abd506991855aaea9bd7e4633..b2f2c36b7223b87b53266ed121c3bdc9bc1a27ed 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 186ef9b56d60b6385fde3336316bc0462008c474..ce3e28e003eafc8f671dde5acab5462a5c57847f 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 906d7c70d6feb8217f65a375dbfea80499eef622..c19c5ffc3671d7fdd45d581aea4032d949f182cb 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 f006d13f09b604738eb45f6f869c539456a20509..c4e9d8ba9bdeb30778fcba3cecd98b4a63335e25 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 640007c705e125876e520fa87a331ecb389b0c41..a49f6db56e8b75307e7790ae1601edb08c2f6b10 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 ae1f4f75d3fa30130151689598ab8dadd6ad5120..22c7917943524ed3c3ee06415cc3159b3ab61892 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 8747d9ba90b01cf26b4cf07cf9b97e5f4827e922..8fdae272746c0b23f7c31fb96cdd96a75cfdc983 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")))