Skip to content

Commit

Permalink
Fixed various signedness issues identified by -Wextra.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tokkee committed Feb 11, 2009
1 parent b72d521 commit f50ada1
Show file tree
Hide file tree
Showing 20 changed files with 76 additions and 62 deletions.
2 changes: 1 addition & 1 deletion src/apache.c
Expand Up @@ -148,7 +148,7 @@ static int init (void)


status = ssnprintf (credentials, sizeof (credentials), "%s:%s", status = ssnprintf (credentials, sizeof (credentials), "%s:%s",
user, (pass == NULL) ? "" : pass); user, (pass == NULL) ? "" : pass);
if (status >= sizeof (credentials)) if ((status < 0) || ((size_t) status >= sizeof (credentials)))
{ {
ERROR ("apache plugin: init: Returning an error " ERROR ("apache plugin: init: Returning an error "
"because the credentials have been " "because the credentials have been "
Expand Down
10 changes: 5 additions & 5 deletions src/ascent.c
Expand Up @@ -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) /* {{{ */ static int ascent_submit_players (player_stats_t *ps) /* {{{ */
{ {
int i; size_t i;
gauge_t value; gauge_t value;


for (i = 0; i < RACES_LIST_LENGTH; i++) for (i = 0; i < RACES_LIST_LENGTH; i++)
Expand Down Expand Up @@ -213,7 +213,7 @@ static int ascent_account_player (player_stats_t *ps, /* {{{ */
{ {
if (pi->race >= 0) if (pi->race >= 0)
{ {
if ((pi->race >= RACES_LIST_LENGTH) if (((size_t) pi->race >= RACES_LIST_LENGTH)
|| (races_list[pi->race] == NULL)) || (races_list[pi->race] == NULL))
ERROR ("ascent plugin: Ignoring invalid numeric race %i.", pi->race); ERROR ("ascent plugin: Ignoring invalid numeric race %i.", pi->race);
else else
Expand All @@ -222,7 +222,7 @@ static int ascent_account_player (player_stats_t *ps, /* {{{ */


if (pi->class >= 0) if (pi->class >= 0)
{ {
if ((pi->class >= CLASSES_LIST_LENGTH) if (((size_t) pi->class >= CLASSES_LIST_LENGTH)
|| (classes_list[pi->class] == NULL)) || (classes_list[pi->class] == NULL))
ERROR ("ascent plugin: Ignoring invalid numeric class %i.", pi->class); ERROR ("ascent plugin: Ignoring invalid numeric class %i.", pi->class);
else else
Expand All @@ -231,7 +231,7 @@ static int ascent_account_player (player_stats_t *ps, /* {{{ */


if (pi->gender >= 0) if (pi->gender >= 0)
{ {
if ((pi->gender >= GENDERS_LIST_LENGTH) if (((size_t) pi->gender >= GENDERS_LIST_LENGTH)
|| (genders_list[pi->gender] == NULL)) || (genders_list[pi->gender] == NULL))
ERROR ("ascent plugin: Ignoring invalid numeric gender %i.", ERROR ("ascent plugin: Ignoring invalid numeric gender %i.",
pi->gender); pi->gender);
Expand Down Expand Up @@ -549,7 +549,7 @@ static int ascent_init (void) /* {{{ */


status = ssnprintf (credentials, sizeof (credentials), "%s:%s", status = ssnprintf (credentials, sizeof (credentials), "%s:%s",
user, (pass == NULL) ? "" : pass); 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 " ERROR ("ascent plugin: ascent_init: Returning an error because the "
"credentials have been truncated."); "credentials have been truncated.");
Expand Down
10 changes: 5 additions & 5 deletions src/collectd-nagios.c
Expand Up @@ -165,7 +165,7 @@ static int filter_ds (size_t *values_num,
return (RET_UNKNOWN); 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; size_t j;


Expand Down Expand Up @@ -298,7 +298,7 @@ static int do_check_con_none (size_t values_num,
int num_okay = 0; int num_okay = 0;
const char *status_str = "UNKNOWN"; const char *status_str = "UNKNOWN";
int status_code = RET_UNKNOWN; int status_code = RET_UNKNOWN;
int i; size_t i;


for (i = 0; i < values_num; i++) for (i = 0; i < values_num; i++)
{ {
Expand Down Expand Up @@ -349,7 +349,7 @@ static int do_check_con_none (size_t values_num,
static int do_check_con_average (size_t values_num, static int do_check_con_average (size_t values_num,
double *values, char **values_names) double *values, char **values_names)
{ {
int i; size_t i;
double total; double total;
int total_num; int total_num;
double average; double average;
Expand Down Expand Up @@ -402,7 +402,7 @@ static int do_check_con_average (size_t values_num,
static int do_check_con_sum (size_t values_num, static int do_check_con_sum (size_t values_num,
double *values, char **values_names) double *values, char **values_names)
{ {
int i; size_t i;
double total; double total;
int total_num; int total_num;
const char *status_str = "UNKNOWN"; const char *status_str = "UNKNOWN";
Expand Down Expand Up @@ -452,7 +452,7 @@ static int do_check_con_sum (size_t values_num,
static int do_check_con_percentage (size_t values_num, static int do_check_con_percentage (size_t values_num,
double *values, char **values_names) double *values, char **values_names)
{ {
int i; size_t i;
double sum = 0.0; double sum = 0.0;
double percentage; double percentage;


Expand Down
2 changes: 1 addition & 1 deletion src/common.c
Expand Up @@ -411,7 +411,7 @@ int check_create_dir (const char *file_orig)
char *saveptr; char *saveptr;
int last_is_file = 1; int last_is_file = 1;
int path_is_absolute = 0; int path_is_absolute = 0;
int len; size_t len;
int i; int i;


/* /*
Expand Down
4 changes: 2 additions & 2 deletions src/configfile.c
Expand Up @@ -555,7 +555,7 @@ static oconfig_item_t *cf_read_dir (const char *dir, int depth)


status = ssnprintf (name, sizeof (name), "%s/%s", status = ssnprintf (name, sizeof (name), "%s/%s",
dir, de->d_name); dir, de->d_name);
if (status >= sizeof (name)) if ((status < 0) || ((size_t) status >= sizeof (name)))
{ {
ERROR ("configfile: Not including `%s/%s' because its" ERROR ("configfile: Not including `%s/%s' because its"
" name is too long.", " name is too long.",
Expand Down Expand Up @@ -630,7 +630,7 @@ static oconfig_item_t *cf_read_generic (const char *path, int depth)
int status; int status;
const char *path_ptr; const char *path_ptr;
wordexp_t we; wordexp_t we;
int i; size_t i;


if (depth >= CF_MAX_DEPTH) if (depth >= CF_MAX_DEPTH)
{ {
Expand Down
9 changes: 5 additions & 4 deletions src/netlink.c
Expand Up @@ -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 /* Update the `iflist'. It's used to know which interfaces exist and query
* them later for qdiscs and classes. */ * 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; char **temp;


Expand Down Expand Up @@ -359,7 +359,8 @@ static int qos_filter (const struct sockaddr_nl __attribute__((unused)) *sa,
return (0); 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 " ERROR ("netlink plugin: qos_filter: msg->tcm_ifindex = %i "
">= iflist_len = %zu", ">= iflist_len = %zu",
Expand Down Expand Up @@ -580,9 +581,9 @@ static int ir_read (void)


/* `link_filter' will update `iflist' which is used here to iterate over all /* `link_filter' will update `iflist' which is used here to iterate over all
* interfaces. */ * 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) if (iflist[ifindex] == NULL)
continue; continue;
Expand Down
9 changes: 5 additions & 4 deletions src/network.c
Expand Up @@ -498,7 +498,7 @@ static int parse_part_values (void **ret_buffer, int *ret_buffer_len,


exp_size = 3 * sizeof (uint16_t) exp_size = 3 * sizeof (uint16_t)
+ pkg_numval * (sizeof (uint8_t) + sizeof (value_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: " WARNING ("network plugin: parse_part_values: "
"Packet too short: " "Packet too short: "
Expand Down Expand Up @@ -562,7 +562,7 @@ static int parse_part_number (void **ret_buffer, int *ret_buffer_len,
uint16_t pkg_length; uint16_t pkg_length;
uint16_t pkg_type; 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: " WARNING ("network plugin: parse_part_number: "
"Packet too short: " "Packet too short: "
Expand Down Expand Up @@ -602,7 +602,7 @@ static int parse_part_string (void **ret_buffer, int *ret_buffer_len,
uint16_t pkg_length; uint16_t pkg_length;
uint16_t pkg_type; 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: " WARNING ("network plugin: parse_part_string: "
"Packet too short: " "Packet too short: "
Expand Down Expand Up @@ -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. /* Check that the package data fits into the output buffer.
* The previous if-statement ensures that: * The previous if-statement ensures that:
* `pkg_length > header_size' */ * `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: " WARNING ("network plugin: parse_part_string: "
"Output buffer too small."); "Output buffer too small.");
Expand Down
5 changes: 3 additions & 2 deletions src/nginx.c
Expand Up @@ -123,8 +123,9 @@ static int init (void)


if (user != NULL) if (user != NULL)
{ {
if (ssnprintf (credentials, sizeof (credentials), int status = ssnprintf (credentials, sizeof (credentials),
"%s:%s", user, pass == NULL ? "" : pass) >= sizeof (credentials)) "%s:%s", user, pass == NULL ? "" : pass);
if ((status < 0) || ((size_t) status >= sizeof (credentials)))
{ {
ERROR ("nginx plugin: Credentials would have been truncated."); ERROR ("nginx plugin: Credentials would have been truncated.");
return (-1); return (-1);
Expand Down
10 changes: 6 additions & 4 deletions src/plugin.c
Expand Up @@ -328,6 +328,7 @@ int plugin_load (const char *type)
int ret; int ret;
struct stat statbuf; struct stat statbuf;
struct dirent *de; struct dirent *de;
int status;


DEBUG ("type = %s", type); DEBUG ("type = %s", type);


Expand All @@ -336,8 +337,8 @@ int plugin_load (const char *type)


/* `cpu' should not match `cpufreq'. To solve this we add `.so' to the /* `cpu' should not match `cpufreq'. To solve this we add `.so' to the
* type when matching the filename */ * type when matching the filename */
if (ssnprintf (typename, sizeof (typename), status = ssnprintf (typename, sizeof (typename), "%s.so", type);
"%s.so", type) >= sizeof (typename)) if ((status < 0) || ((size_t) status >= sizeof (typename)))
{ {
WARNING ("snprintf: truncated: `%s.so'", type); WARNING ("snprintf: truncated: `%s.so'", type);
return (-1); return (-1);
Expand All @@ -357,8 +358,9 @@ int plugin_load (const char *type)
if (strncasecmp (de->d_name, typename, typename_len)) if (strncasecmp (de->d_name, typename, typename_len))
continue; continue;


if (ssnprintf (filename, sizeof (filename), status = ssnprintf (filename, sizeof (filename),
"%s/%s", dir, de->d_name) >= 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); WARNING ("snprintf: truncated: `%s/%s'", dir, de->d_name);
continue; continue;
Expand Down
2 changes: 1 addition & 1 deletion src/processes.c
Expand Up @@ -821,7 +821,7 @@ static char *ps_get_cmdline (pid_t pid, char *name, char *buf, size_t buf_len)
n = 0; n = 0;


while (42) { while (42) {
size_t status; ssize_t status;


status = read (fd, (void *)buf_ptr, len); status = read (fd, (void *)buf_ptr, len);


Expand Down
12 changes: 6 additions & 6 deletions src/snmp.c
Expand Up @@ -530,9 +530,9 @@ static int csnmp_config_add_host_interval (host_definition_t *hd, oconfig_item_t
return (-1); return (-1);
} }


hd->interval = (int) ci->values[0].value.number; hd->interval = ci->values[0].value.number >= 0
if (hd->interval < 0) ? (uint32_t) ci->values[0].value.number
hd->interval = 0; : 0;


return (0); return (0);
} /* int csnmp_config_add_host_interval */ } /* int csnmp_config_add_host_interval */
Expand Down Expand Up @@ -1138,7 +1138,7 @@ static int csnmp_read_table (host_definition_t *host, data_definition_t *data)
break; 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); snmp_add_null_var (req, oid_list[i].oid, oid_list[i].oid_len);


res = NULL; res = NULL;
Expand Down Expand Up @@ -1441,7 +1441,7 @@ static int csnmp_read_host (host_definition_t *host)
time_end = time (NULL); time_end = time (NULL);
DEBUG ("snmp plugin: csnmp_read_host (%s) finished at %u;", host->name, DEBUG ("snmp plugin: csnmp_read_host (%s) finished at %u;", host->name,
(unsigned int) time_end); (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, " WARNING ("snmp plugin: Host `%s' should be queried every %i seconds, "
"but reading all values takes %u seconds.", "but reading all values takes %u seconds.",
Expand Down Expand Up @@ -1504,7 +1504,7 @@ static int csnmp_init (void)
{ {
host->interval = interval_g; host->interval = interval_g;
} }
else if (host->interval < interval_g) else if (host->interval < (uint32_t) interval_g)
{ {
host->interval = interval_g; host->interval = interval_g;
WARNING ("snmp plugin: Data for host `%s' will be collected every %i seconds.", WARNING ("snmp plugin: Data for host `%s' will be collected every %i seconds.",
Expand Down
4 changes: 2 additions & 2 deletions src/tail.c
Expand Up @@ -305,7 +305,7 @@ static int ctail_init (void)
static int ctail_read (void) static int ctail_read (void)
{ {
int success = 0; int success = 0;
int i; size_t i;


for (i = 0; i < tail_match_list_num; i++) for (i = 0; i < tail_match_list_num; i++)
{ {
Expand All @@ -329,7 +329,7 @@ static int ctail_read (void)


static int ctail_shutdown (void) static int ctail_shutdown (void)
{ {
int i; size_t i;


for (i = 0; i < tail_match_list_num; i++) for (i = 0; i < tail_match_list_num; i++)
{ {
Expand Down
19 changes: 12 additions & 7 deletions src/thermal.c
Expand Up @@ -71,8 +71,9 @@ static int thermal_sysfs_device_read (const char __attribute__((unused)) *dir,
if (device_list && ignorelist_match (device_list, name)) if (device_list && ignorelist_match (device_list, name))
return -1; return -1;


len = snprintf (filename, sizeof (filename), "%s/%s/temp", dirname_sysfs, name); len = snprintf (filename, sizeof (filename),
if ((len < 0) || ((unsigned int)len >= sizeof (filename))) "%s/%s/temp", dirname_sysfs, name);
if ((len < 0) || ((size_t) len >= sizeof (filename)))
return -1; return -1;


len = read_file_contents (filename, data, sizeof(data)); len = read_file_contents (filename, data, sizeof(data));
Expand All @@ -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); len = snprintf (filename, sizeof (filename),
if ((len < 0) || ((unsigned int)len >= sizeof (filename))) "%s/%s/cur_state", dirname_sysfs, name);
if ((len < 0) || ((size_t) len >= sizeof (filename)))
return -1; return -1;


len = read_file_contents (filename, data, sizeof(data)); len = read_file_contents (filename, data, sizeof(data));
Expand Down Expand Up @@ -128,12 +130,15 @@ static int thermal_procfs_device_read (const char __attribute__((unused)) *dir,
* temperature: 55 C * temperature: 55 C
*/ */


len = snprintf (filename, sizeof (filename), "%s/%s/temperature", dirname_procfs, name); len = snprintf (filename, sizeof (filename),
if ((len < 0) || ((unsigned int)len >= sizeof (filename))) "%s/%s/temperature", dirname_procfs, name);
if ((len < 0) || ((size_t) len >= sizeof (filename)))
return -1; return -1;


len = read_file_contents (filename, data, sizeof(data)); 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; char *endptr = NULL;
double temp; double temp;
double celsius, add; double celsius, add;
Expand Down
2 changes: 1 addition & 1 deletion src/utils_cache.c
Expand Up @@ -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 /* This is important - the caller has no other way of knowing how many
* values are returned. */ * 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, " ERROR ("utils_cache: uc_get_rate: ds[%s] has %i values, "
"but uc_get_rate_by_name returned %zu.", "but uc_get_rate_by_name returned %zu.",
Expand Down
4 changes: 2 additions & 2 deletions src/utils_cmd_getval.c
Expand Up @@ -51,7 +51,7 @@ int handle_getval (FILE *fh, char *buffer)
const data_set_t *ds; const data_set_t *ds;


int status; int status;
int i; size_t i;


if ((fh == NULL) || (buffer == NULL)) if ((fh == NULL) || (buffer == NULL))
return (-1); return (-1);
Expand Down Expand Up @@ -123,7 +123,7 @@ int handle_getval (FILE *fh, char *buffer)
return (-1); return (-1);
} }


if (ds->ds_num != values_num) if ((size_t) ds->ds_num != values_num)
{ {
ERROR ("ds[%s]->ds_num = %i, " ERROR ("ds[%s]->ds_num = %i, "
"but uc_get_rate_by_name returned %u values.", "but uc_get_rate_by_name returned %u values.",
Expand Down
4 changes: 2 additions & 2 deletions src/utils_match.c
Expand Up @@ -50,7 +50,7 @@ static char *match_substr (const char *str, int begin, int end)


if ((begin < 0) || (end < 0) || (begin >= end)) if ((begin < 0) || (end < 0) || (begin >= end))
return (NULL); 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."); ERROR ("utils_match: match_substr: `end' points after end of string.");
return (NULL); return (NULL);
Expand Down Expand Up @@ -228,7 +228,7 @@ int match_apply (cu_match_t *obj, const char *str)
regmatch_t re_match[32]; regmatch_t re_match[32];
char *matches[32]; char *matches[32];
size_t matches_num; size_t matches_num;
int i; size_t i;


if ((obj == NULL) || (str == NULL)) if ((obj == NULL) || (str == NULL))
return (-1); return (-1);
Expand Down

0 comments on commit f50ada1

Please sign in to comment.