summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: b72d521)
raw | patch | inline | side by side (parent: b72d521)
author | Sebastian Harl <sh@tokkee.org> | |
Wed, 11 Feb 2009 10:31:30 +0000 (11:31 +0100) | ||
committer | Sebastian 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.
* 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 | patch | blob | history | |
src/ascent.c | patch | blob | history | |
src/collectd-nagios.c | patch | blob | history | |
src/common.c | patch | blob | history | |
src/configfile.c | patch | blob | history | |
src/netlink.c | patch | blob | history | |
src/network.c | patch | blob | history | |
src/nginx.c | patch | blob | history | |
src/plugin.c | patch | blob | history | |
src/processes.c | patch | blob | history | |
src/snmp.c | patch | blob | history | |
src/tail.c | patch | blob | history | |
src/thermal.c | patch | blob | history | |
src/utils_cache.c | patch | blob | history | |
src/utils_cmd_getval.c | patch | blob | history | |
src/utils_match.c | patch | blob | history | |
src/utils_rrdcreate.c | patch | blob | history | |
src/utils_subst.c | patch | blob | history | |
src/utils_tail_match.c | patch | blob | history | |
src/vserver.c | patch | blob | history |
diff --git a/src/apache.c b/src/apache.c
index 091d594741612e3f12d99f3852af58bf90785fbd..4fa7aa1b11a9f8dbf192271abfe65322be0e3806 100644 (file)
--- a/src/apache.c
+++ b/src/apache.c
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 54341c7733cfa116cfe79d463eb7fec81159f50e..8829e518f3fbfef289b9d74d368fe7ffc1de7bbf 100644 (file)
--- 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++)
{
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
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
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);
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 b995b1c58efa28af61a912a7bb86b24597788e31..8f4a41054ab55efbf0e79bf4d2a51b3da0e9244e 100644 (file)
--- a/src/collectd-nagios.c
+++ b/src/collectd-nagios.c
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;
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++)
{
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;
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";
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 7cc1d9d44e591f5a93a917016752350cf2d2a649..28f16da0190ada6346806335f8f0ba484afc7f72 100644 (file)
--- a/src/common.c
+++ b/src/common.c
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 0f49de266f643929632895ca4d76b7630732bb60..bb1770d2ab13e8c8a26c84901dd9e012dd0fc9a7 100644 (file)
--- a/src/configfile.c
+++ b/src/configfile.c
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.",
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 8f45ea2e79334927b3b29942a3f33296bb9f2a6e..b15768e719d5c7ea847848ea63db4142e703c45c 100644 (file)
--- a/src/netlink.c
+++ b/src/netlink.c
/* 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;
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",
/* `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 e080000442f25f58aa5dda6f8b4c3a0b38d555bd..66f04380388fc3ad20d56d3f95333567e55bebfe 100644 (file)
--- a/src/network.c
+++ b/src/network.c
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: "
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: "
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: "
/* 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 dfa4f9832f0252acdfe789d5effc7e560ebcaa4c..2de3633b41c6ffdc2feb1ffc64d329bca7092f9b 100644 (file)
--- a/src/nginx.c
+++ b/src/nginx.c
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 9d558cb1941f54df06ed2523398c0ae61abd4c62..61cc09c6251500c275ddee80582cd7bcbb8f7bab 100644 (file)
--- a/src/plugin.c
+++ b/src/plugin.c
int ret;
struct stat statbuf;
struct dirent *de;
+ int status;
DEBUG ("type = %s", 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);
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 553b1955c2f868904f728f315203bdbb848327c4..26dde3e4f43e91d29ae4955e66eae705ebdd1409 100644 (file)
--- a/src/processes.c
+++ b/src/processes.c
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 82fd6583b075b5f243d06d6e85b9cb86803302a0..998c4b648fbffde71a2dc9cd5f056b80ff266df4 100644 (file)
--- 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;
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.",
{
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 01bf6292ac2d1583796d42d387a69b4080d6752f..3a98c60f0c67436c9ea9804d9bf272a925eda568 100644 (file)
--- a/src/tail.c
+++ b/src/tail.c
static int ctail_read (void)
{
int success = 0;
- int i;
+ size_t i;
for (i = 0; i < tail_match_list_num; i++)
{
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 2ea6a24fb916719937376556f0bc8d3f6d3d7f41..fe54aee443790617b30c22915414bcd4c5123b5c 100644 (file)
--- a/src/thermal.c
+++ b/src/thermal.c
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));
}
}
- 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 824b7c80bcf6d64abd506991855aaea9bd7e4633..b2f2c36b7223b87b53266ed121c3bdc9bc1a27ed 100644 (file)
--- a/src/utils_cache.c
+++ b/src/utils_cache.c
/* 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 186ef9b56d60b6385fde3336316bc0462008c474..ce3e28e003eafc8f671dde5acab5462a5c57847f 100644 (file)
--- a/src/utils_cmd_getval.c
+++ b/src/utils_cmd_getval.c
const data_set_t *ds;
int status;
- int i;
+ size_t i;
if ((fh == NULL) || (buffer == NULL))
return (-1);
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 906d7c70d6feb8217f65a375dbfea80499eef622..c19c5ffc3671d7fdd45d581aea4032d949f182cb 100644 (file)
--- a/src/utils_match.c
+++ b/src/utils_match.c
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);
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 f006d13f09b604738eb45f6f869c539456a20509..c4e9d8ba9bdeb30778fcba3cecd98b4a63335e25 100644 (file)
--- a/src/utils_rrdcreate.c
+++ b/src/utils_rrdcreate.c
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;
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 640007c705e125876e520fa87a331ecb389b0c41..a49f6db56e8b75307e7790ae1601edb08c2f6b10 100644 (file)
--- a/src/utils_subst.c
+++ b/src/utils_subst.c
|| (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 ae1f4f75d3fa30130151689598ab8dadd6ad5120..22c7917943524ed3c3ee06415cc3159b3ab61892 100644 (file)
--- a/src/utils_tail_match.c
+++ b/src/utils_tail_match.c
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);
void tail_match_destroy (cu_tail_match_t *obj)
{
- int i;
+ size_t i;
if (obj == NULL)
return;
{
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 8747d9ba90b01cf26b4cf07cf9b97e5f4827e922..8fdae272746c0b23f7c31fb96cdd96a75cfdc983 100644 (file)
--- a/src/vserver.c
+++ b/src/vserver.c
while (42)
{
- size_t len;
+ int len;
char file[BUFSIZE];
FILE *fh;
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;
/* 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")))
/* 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")))
/* 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")))