From f50ada19dcd0dc6185dc4b410e721d46d1caba9f Mon Sep 17 00:00:00 2001 From: Sebastian Harl Date: Wed, 11 Feb 2009 11:31:30 +0100 Subject: [PATCH] Fixed various signedness issues identified by -Wextra. 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. --- src/apache.c | 2 +- src/ascent.c | 10 +++++----- src/collectd-nagios.c | 10 +++++----- src/common.c | 2 +- src/configfile.c | 4 ++-- src/netlink.c | 9 +++++---- src/network.c | 9 +++++---- src/nginx.c | 5 +++-- src/plugin.c | 10 ++++++---- src/processes.c | 2 +- src/snmp.c | 12 ++++++------ src/tail.c | 4 ++-- src/thermal.c | 19 ++++++++++++------- src/utils_cache.c | 2 +- src/utils_cmd_getval.c | 4 ++-- src/utils_match.c | 4 ++-- src/utils_rrdcreate.c | 11 +++++++---- src/utils_subst.c | 3 ++- src/utils_tail_match.c | 6 +++--- src/vserver.c | 10 +++++----- 20 files changed, 76 insertions(+), 62 deletions(-) diff --git a/src/apache.c b/src/apache.c index 091d5947..4fa7aa1b 100644 --- a/src/apache.c +++ b/src/apache.c @@ -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 " diff --git a/src/ascent.c b/src/ascent.c index 54341c77..8829e518 100644 --- a/src/ascent.c +++ b/src/ascent.c @@ -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."); diff --git a/src/collectd-nagios.c b/src/collectd-nagios.c index b995b1c5..8f4a4105 100644 --- a/src/collectd-nagios.c +++ b/src/collectd-nagios.c @@ -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; diff --git a/src/common.c b/src/common.c index 7cc1d9d4..28f16da0 100644 --- a/src/common.c +++ b/src/common.c @@ -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; /* diff --git a/src/configfile.c b/src/configfile.c index 0f49de26..bb1770d2 100644 --- a/src/configfile.c +++ b/src/configfile.c @@ -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) { diff --git a/src/netlink.c b/src/netlink.c index 8f45ea2e..b15768e7 100644 --- a/src/netlink.c +++ b/src/netlink.c @@ -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; diff --git a/src/network.c b/src/network.c index e0800004..66f04380 100644 --- a/src/network.c +++ b/src/network.c @@ -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."); diff --git a/src/nginx.c b/src/nginx.c index dfa4f983..2de3633b 100644 --- a/src/nginx.c +++ b/src/nginx.c @@ -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); diff --git a/src/plugin.c b/src/plugin.c index 9d558cb1..61cc09c6 100644 --- a/src/plugin.c +++ b/src/plugin.c @@ -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; diff --git a/src/processes.c b/src/processes.c index 553b1955..26dde3e4 100644 --- a/src/processes.c +++ b/src/processes.c @@ -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); diff --git a/src/snmp.c b/src/snmp.c index 82fd6583..998c4b64 100644 --- a/src/snmp.c +++ b/src/snmp.c @@ -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.", diff --git a/src/tail.c b/src/tail.c index 01bf6292..3a98c60f 100644 --- a/src/tail.c +++ b/src/tail.c @@ -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++) { diff --git a/src/thermal.c b/src/thermal.c index 2ea6a24f..fe54aee4 100644 --- a/src/thermal.c +++ b/src/thermal.c @@ -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; diff --git a/src/utils_cache.c b/src/utils_cache.c index 824b7c80..b2f2c36b 100644 --- a/src/utils_cache.c +++ b/src/utils_cache.c @@ -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.", diff --git a/src/utils_cmd_getval.c b/src/utils_cmd_getval.c index 186ef9b5..ce3e28e0 100644 --- a/src/utils_cmd_getval.c +++ b/src/utils_cmd_getval.c @@ -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.", diff --git a/src/utils_match.c b/src/utils_match.c index 906d7c70..c19c5ffc 100644 --- a/src/utils_match.c +++ b/src/utils_match.c @@ -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); diff --git a/src/utils_rrdcreate.c b/src/utils_rrdcreate.c index f006d13f..c4e9d8ba 100644 --- a/src/utils_rrdcreate.c +++ b/src/utils_rrdcreate.c @@ -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); diff --git a/src/utils_subst.c b/src/utils_subst.c index 640007c7..a49f6db5 100644 --- a/src/utils_subst.c +++ b/src/utils_subst.c @@ -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; diff --git a/src/utils_tail_match.c b/src/utils_tail_match.c index ae1f4f75..22c79179 100644 --- a/src/utils_tail_match.c +++ b/src/utils_tail_match.c @@ -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); diff --git a/src/vserver.c b/src/vserver.c index 8747d9ba..8fdae272 100644 --- a/src/vserver.c +++ b/src/vserver.c @@ -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"))) -- 2.11.0