[collectd] [PATCH] Fixed some compiler warnings identified by gcc's -Wextra option.

Sebastian Harl sh at tokkee.org
Tue Jan 22 19:11:00 CET 2008


The following issues have been addressed:

 * comparison between signed and unsigned - this was found in several places
   throughout the code and has been fixed in various ways
 * missing initializer - an incomplete initializer has been used for two
   struct instances in perl.c
 * unused parameter - when applicable, the parameter has been removed; in
   thirteen cases the parameter is required by different library API's and in
   two cases the parameter was left in place to retain a consistent interface
   within the affected modules; as __attribute__((unused)) is a GNU extension,
   it has not been used to document those exceptions

Signed-off-by: Sebastian Harl <sh at tokkee.org>
---
 src/apcups.c        |    6 +++---
 src/battery.c       |    8 ++++----
 src/collectd.c      |    6 +++---
 src/collectdmon.c   |    6 +++---
 src/common.c        |    8 ++++----
 src/cpufreq.c       |    4 ++--
 src/disk.c          |    5 ++---
 src/iptables.c      |    6 +++---
 src/irq.c           |    4 ++--
 src/network.c       |    3 ++-
 src/ntpd.c          |    4 ++--
 src/perl.c          |    6 +++---
 src/plugin.c        |    2 +-
 src/plugin.h        |    2 +-
 src/types_list.c    |    4 ++--
 src/unixsock.c      |    6 +++---
 src/utils_avltree.c |   12 ++++++------
 src/utils_dns.c     |   11 +++++++----
 18 files changed, 53 insertions(+), 50 deletions(-)

diff --git a/src/apcups.c b/src/apcups.c
index 08df0b5..5a03764 100644
--- a/src/apcups.c
+++ b/src/apcups.c
@@ -112,7 +112,7 @@ static int apcups_shutdown (void)
  * Returns -1 on error
  * Returns socket file descriptor otherwise
  */
-static int net_open (char *host, char *service, int port)
+static int net_open (char *host, int port)
 {
 	int              sd;
 	int              status;
@@ -270,7 +270,7 @@ static int apc_query_server (char *host, int port,
 
 	if (global_sockfd < 0)
 	{
-		global_sockfd = net_open (host, NULL, port);
+		global_sockfd = net_open (host, port);
 		if (global_sockfd < 0)
 		{
 			ERROR ("apcups plugin: Connecting to the "
@@ -287,7 +287,7 @@ static int apc_query_server (char *host, int port,
 
 	while ((n = net_recv (&global_sockfd, recvline, sizeof (recvline) - 1)) > 0)
 	{
-		assert (n < sizeof (recvline));
+		assert ((unsigned int)n < sizeof (recvline));
 		recvline[n] = '\0';
 #if APCMAIN
 		printf ("net_recv = `%s';\n", recvline);
diff --git a/src/battery.c b/src/battery.c
index 2e27e60..345f606 100644
--- a/src/battery.c
+++ b/src/battery.c
@@ -77,7 +77,7 @@ static int battery_init (void)
 	{
 		len = snprintf (filename, sizeof (filename), battery_pmu_file, battery_pmu_num);
 
-		if ((len >= sizeof (filename)) || (len < 0))
+		if ((len < 0) || ((unsigned int)len >= sizeof (filename)))
 			break;
 
 		if (access (filename, R_OK))
@@ -360,11 +360,11 @@ static int battery_read (void)
 		double *valptr = NULL;
 
 		len = snprintf (filename, sizeof (filename), battery_pmu_file, i);
-		if ((len >= sizeof (filename)) || (len < 0))
+		if ((len < 0) || ((unsigned int)len >= sizeof (filename)))
 			continue;
 
 		len = snprintf (batnum_str, sizeof (batnum_str), "%i", i);
-		if ((len >= sizeof (batnum_str)) || (len < 0))
+		if ((len < 0) || ((unsigned int)len >= sizeof (batnum_str)))
 			continue;
 
 		if ((fh = fopen (filename, "r")) == NULL)
@@ -438,7 +438,7 @@ static int battery_read (void)
 			len = snprintf (filename, sizeof (filename),
 					"/proc/acpi/battery/%s/state",
 					ent->d_name);
-			if ((len >= sizeof (filename)) || (len < 0))
+			if ((len < 0) || ((unsigned int)len >= sizeof (filename)))
 				continue;
 
 			if ((fh = fopen (filename, "r")) == NULL)
diff --git a/src/collectd.c b/src/collectd.c
index c9ae66d..2d161aa 100644
--- a/src/collectd.c
+++ b/src/collectd.c
@@ -215,7 +215,7 @@ static void update_kstat (void)
 /* TODO
  * Remove all settings but `-f' and `-C'
  */
-static void exit_usage (char *name)
+static void exit_usage (void)
 {
 	printf ("Usage: "PACKAGE" [OPTIONS]\n\n"
 			
@@ -288,7 +288,7 @@ static int do_loop (void)
 #endif
 
 		/* Issue all plugins */
-		plugin_read_all (&loop);
+		plugin_read_all ();
 
 		if (gettimeofday (&tv_now, NULL) < 0)
 		{
@@ -404,7 +404,7 @@ int main (int argc, char **argv)
 #endif /* COLLECT_DAEMON */
 			case 'h':
 			default:
-				exit_usage (argv[0]);
+				exit_usage ();
 		} /* switch (c) */
 	} /* while (1) */
 
diff --git a/src/collectdmon.c b/src/collectdmon.c
index 0295ad3..e496eb0 100644
--- a/src/collectdmon.c
+++ b/src/collectdmon.c
@@ -140,7 +140,7 @@ static int daemonize (void)
 	if (RLIM_INFINITY == rl.rlim_max)
 		rl.rlim_max = 1024;
 
-	for (i = 0; i < rl.rlim_max; ++i)
+	for (i = 0; i < (int)rl.rlim_max; ++i)
 		close (i);
 
 	errno = 0;
@@ -166,7 +166,7 @@ static int daemonize (void)
 	return 0;
 } /* daemonize */
 
-static int collectd_start (int argc, char **argv)
+static int collectd_start (char **argv)
 {
 	pid_t pid = 0;
 
@@ -340,7 +340,7 @@ int main (int argc, char **argv)
 	while (0 == loop) {
 		int status = 0;
 
-		if (0 != collectd_start (collectd_argc, collectd_argv)) {
+		if (0 != collectd_start (collectd_argv)) {
 			syslog (LOG_ERR, "Error: failed to start collectd.");
 			break;
 		}
diff --git a/src/common.c b/src/common.c
index 1138f96..b1fd953 100644
--- a/src/common.c
+++ b/src/common.c
@@ -176,7 +176,7 @@ ssize_t sread (int fd, void *buf, size_t count)
 			return (-1);
 		}
 
-		assert (nleft >= status);
+		assert ((0 > status) || (nleft >= (size_t)status));
 
 		nleft = nleft - status;
 		ptr   = ptr   + status;
@@ -237,8 +237,8 @@ int strjoin (char *dst, size_t dst_len,
 		char **fields, size_t fields_num,
 		const char *sep)
 {
-	int field_len;
-	int sep_len;
+	size_t field_len;
+	size_t sep_len;
 	int i;
 
 	memset (dst, '\0', dst_len);
@@ -250,7 +250,7 @@ int strjoin (char *dst, size_t dst_len,
 	if (sep != NULL)
 		sep_len = strlen (sep);
 
-	for (i = 0; i < fields_num; i++)
+	for (i = 0; i < (int)fields_num; i++)
 	{
 		if ((i > 0) && (sep_len > 0))
 		{
diff --git a/src/cpufreq.c b/src/cpufreq.c
index b1037c3..42248a9 100644
--- a/src/cpufreq.c
+++ b/src/cpufreq.c
@@ -40,7 +40,7 @@ static int cpufreq_init (void)
 		status = snprintf (filename, sizeof (filename),
 				"/sys/devices/system/cpu/cpu%d/cpufreq/"
 				"scaling_cur_freq", num_cpu);
-    		if (status < 1 || status >= sizeof (filename))
+		if ((status < 1) || ((unsigned int)status >= sizeof (filename)))
 			break;
 
 		if (access (filename, R_OK))
@@ -90,7 +90,7 @@ static int cpufreq_read (void)
 		status = snprintf (filename, sizeof (filename),
 				"/sys/devices/system/cpu/cpu%d/cpufreq/"
 				"scaling_cur_freq", i);
-    		if (status < 1 || status >= sizeof (filename))
+		if ((status < 1) || ((unsigned int)status >= sizeof (filename)))
 			return (-1);
 
 		if ((fp = fopen (filename, "r")) == NULL)
diff --git a/src/disk.c b/src/disk.c
index d2e9f98..8feaa8d 100644
--- a/src/disk.c
+++ b/src/disk.c
@@ -562,9 +562,8 @@ static int disk_read (void)
 
 		if (is_disk)
 		{
-			if ((read_merged != -1LL) || (write_merged != -1LL))
-				disk_submit (disk_name, "disk_merged",
-						read_merged, write_merged);
+			disk_submit (disk_name, "disk_merged",
+					read_merged, write_merged);
 		} /* if (is_disk) */
 	} /* while (fgets (buffer, sizeof (buffer), fh) != NULL) */
 
diff --git a/src/iptables.c b/src/iptables.c
index 0e7fa70..72b4481 100644
--- a/src/iptables.c
+++ b/src/iptables.c
@@ -106,7 +106,7 @@ static int iptables_config (const char *key, const char *value)
 		chain = fields[1];
 
 		table_len = strlen (table);
-		if (table_len >= sizeof(temp.table))
+		if ((unsigned int)table_len >= sizeof(temp.table))
 		{
 			ERROR ("Table `%s' too long.", table);
 			free (value_copy);
@@ -116,7 +116,7 @@ static int iptables_config (const char *key, const char *value)
 		temp.table[table_len] = '\0';
 
 		chain_len = strlen (chain);
-		if (chain_len >= sizeof(temp.chain))
+		if ((unsigned int)chain_len >= sizeof(temp.chain))
 		{
 			ERROR ("Chain `%s' too long.", chain);
 			free (value_copy);
@@ -224,7 +224,7 @@ static int submit_match (const struct ipt_entry_match *match,
 
     status = snprintf (vl.plugin_instance, sizeof (vl.plugin_instance),
 	    "%s-%s", chain->table, chain->chain);
-    if ((status >= sizeof (vl.plugin_instance)) || (status < 1))
+    if ((status < 1) || ((unsigned int)status >= sizeof (vl.plugin_instance)))
 	return (0);
 
     if (chain->name[0] != '\0')
diff --git a/src/irq.c b/src/irq.c
index b6b8c4c..9eb1de4 100644
--- a/src/irq.c
+++ b/src/irq.c
@@ -111,7 +111,7 @@ static int check_ignore_irq (const unsigned int irq)
 	if (irq_list_num < 1)
 		return (0);
 
-	for (i = 0; i < irq_list_num; i++)
+	for (i = 0; (unsigned int)i < irq_list_num; i++)
 		if (irq == irq_list[i])
 			return (irq_list_action);
 
@@ -137,7 +137,7 @@ static void irq_submit (unsigned int irq, counter_t value)
 
 	status = snprintf (vl.type_instance, sizeof (vl.type_instance),
 			"%u", irq);
-	if ((status < 1) || (status >= sizeof (vl.type_instance)))
+	if ((status < 1) || ((unsigned int)status >= sizeof (vl.type_instance)))
 		return;
 
 	plugin_dispatch_values ("irq", &vl);
diff --git a/src/network.c b/src/network.c
index c347552..7fb19a1 100644
--- a/src/network.c
+++ b/src/network.c
@@ -513,7 +513,8 @@ static int parse_packet (void *buffer, int buffer_len)
 	memset (&type, '\0', sizeof (type));
 	status = 0;
 
-	while ((status == 0) && (buffer_len > sizeof (part_header_t)))
+	while ((status == 0) && (0 < buffer_len)
+			&& ((unsigned int)buffer_len > sizeof (part_header_t)))
 	{
 		header = (part_header_t *) buffer;
 
diff --git a/src/ntpd.c b/src/ntpd.c
index c5dcb8e..90fdfd7 100644
--- a/src/ntpd.c
+++ b/src/ntpd.c
@@ -408,7 +408,7 @@ static int ntpd_connect (void)
 }
 
 /* For a description of the arguments see `ntpd_do_query' below. */
-static int ntpd_receive_response (int req_code, int *res_items, int *res_size,
+static int ntpd_receive_response (int *res_items, int *res_size,
 		char **res_data, int res_item_size)
 {
 	int              sd;
@@ -766,7 +766,7 @@ static int ntpd_do_query (int req_code, int req_items, int req_size, char *req_d
 	if (status != 0)
 		return (status);
 
-	status = ntpd_receive_response (req_code, res_items, res_size, res_data,
+	status = ntpd_receive_response (res_items, res_size, res_data,
 			res_item_size);
 	return (status);
 }
diff --git a/src/perl.c b/src/perl.c
index dc548b2..b87d6b7 100644
--- a/src/perl.c
+++ b/src/perl.c
@@ -376,7 +376,7 @@ static char *get_module_name (char *buf, size_t buf_len, const char *module) {
 		status = snprintf (buf, buf_len, "%s", module);
 	else
 		status = snprintf (buf, buf_len, "%s::%s", base_name, module);
-	if ((status < 0) || (status >= buf_len))
+	if ((status < 0) || ((unsigned int)status >= buf_len))
 		return (NULL);
 	buf[buf_len - 1] = '\0';
 	return (buf);
@@ -1089,8 +1089,8 @@ static int g_iv_set (pTHX_ SV *var, MAGIC *mg)
 	return 0;
 } /* static int g_iv_set (pTHX_ SV *, MAGIC *) */
 
-static MGVTBL g_pv_vtbl = { g_pv_get, g_pv_set, NULL, NULL, NULL };
-static MGVTBL g_iv_vtbl = { g_iv_get, g_iv_set, NULL, NULL, NULL };
+static MGVTBL g_pv_vtbl = { g_pv_get, g_pv_set, NULL, NULL, NULL, NULL, NULL };
+static MGVTBL g_iv_vtbl = { g_iv_get, g_iv_set, NULL, NULL, NULL, NULL, NULL };
 
 /* bootstrap the Collectd module */
 static void xs_init (pTHX)
diff --git a/src/plugin.c b/src/plugin.c
index 88da209..77f7aff 100644
--- a/src/plugin.c
+++ b/src/plugin.c
@@ -600,7 +600,7 @@ void plugin_init_all (void)
 	}
 } /* void plugin_init_all */
 
-void plugin_read_all (const int *loop)
+void plugin_read_all (void)
 {
 	llentry_t   *le;
 	read_func_t *rf;
diff --git a/src/plugin.h b/src/plugin.h
index 428c4f0..4918049 100644
--- a/src/plugin.h
+++ b/src/plugin.h
@@ -144,7 +144,7 @@ void plugin_set_dir (const char *dir);
 int plugin_load (const char *name);
 
 void plugin_init_all (void);
-void plugin_read_all (const int *loop);
+void plugin_read_all (void);
 void plugin_shutdown_all (void);
 
 /*
diff --git a/src/types_list.c b/src/types_list.c
index 43e8790..ff84262 100644
--- a/src/types_list.c
+++ b/src/types_list.c
@@ -91,7 +91,7 @@ static int parse_ds (data_source_t *dsrc, char *buf, size_t buf_len)
   return (0);
 } /* int parse_ds */
 
-static void parse_line (char *buf, size_t buf_len)
+static void parse_line (char *buf)
 {
   char  *fields[64];
   size_t fields_num;
@@ -165,7 +165,7 @@ static void parse_file (FILE *fh)
     if (buf_len == 0)
       continue;
 
-    parse_line (buf, buf_len);
+    parse_line (buf);
   } /* while (fgets) */
 } /* void parse_file */
 
diff --git a/src/unixsock.c b/src/unixsock.c
index c7e0c44..3dd0b3d 100644
--- a/src/unixsock.c
+++ b/src/unixsock.c
@@ -81,7 +81,7 @@ static pthread_t listen_thread = (pthread_t) 0;
 /* Linked list and auxilliary variables for saving values */
 static value_cache_t   *cache_head = NULL;
 static pthread_mutex_t  cache_lock = PTHREAD_MUTEX_INITIALIZER;
-static unsigned int     cache_oldest = UINT_MAX;
+static time_t           cache_oldest = -1;
 
 /*
  * Functions
@@ -188,7 +188,7 @@ static int cache_insert (const data_set_t *ds, const value_list_t *vl)
 	cache_head = vc;
 
 	vc->time = vl->time;
-	if (vc->time < cache_oldest)
+	if ((vc->time < cache_oldest) || (-1 == cache_oldest))
 		cache_oldest = vc->time;
 
 	pthread_mutex_unlock (&cache_lock);
@@ -275,7 +275,7 @@ static int cache_update (const data_set_t *ds, const value_list_t *vl)
 	vc->ds = ds;
 	vc->time = vl->time;
 
-	if (vc->time < cache_oldest)
+	if ((vc->time < cache_oldest) || (-1 == cache_oldest))
 		cache_oldest = vc->time;
 
 	pthread_mutex_unlock (&cache_lock);
diff --git a/src/utils_avltree.c b/src/utils_avltree.c
index 6ad0227..9f0b796 100644
--- a/src/utils_avltree.c
+++ b/src/utils_avltree.c
@@ -264,7 +264,7 @@ static void rebalance (c_avl_tree_t *t, c_avl_node_t *n)
 	} /* while (n != NULL) */
 } /* void rebalance */
 
-static c_avl_node_t *c_avl_node_next (c_avl_tree_t *t, c_avl_node_t *n)
+static c_avl_node_t *c_avl_node_next (c_avl_node_t *n)
 {
 	c_avl_node_t *r; /* return node */
 
@@ -309,7 +309,7 @@ static c_avl_node_t *c_avl_node_next (c_avl_tree_t *t, c_avl_node_t *n)
 	return (r);
 } /* c_avl_node_t *c_avl_node_next */
 
-static c_avl_node_t *c_avl_node_prev (c_avl_tree_t *t, c_avl_node_t *n)
+static c_avl_node_t *c_avl_node_prev (c_avl_node_t *n)
 {
 	c_avl_node_t *r; /* return node */
 
@@ -364,13 +364,13 @@ static int _remove (c_avl_tree_t *t, c_avl_node_t *n)
 		if (BALANCE (n) > 0) /* left subtree is higher */
 		{
 			assert (n->left != NULL);
-			r = c_avl_node_prev (t, n);
+			r = c_avl_node_prev (n);
 			
 		}
 		else /* right subtree is higher */
 		{
 			assert (n->right != NULL);
-			r = c_avl_node_next (t, n);
+			r = c_avl_node_next (n);
 		}
 
 		assert ((r->left == NULL) || (r->right == NULL));
@@ -662,7 +662,7 @@ int c_avl_iterator_next (c_avl_iterator_t *iter, void **key, void **value)
 	}
 	else
 	{
-		n = c_avl_node_next (iter->tree, iter->node);
+		n = c_avl_node_next (iter->node);
 	}
 
 	if (n == NULL)
@@ -691,7 +691,7 @@ int c_avl_iterator_prev (c_avl_iterator_t *iter, void **key, void **value)
 	}
 	else
 	{
-		n = c_avl_node_prev (iter->tree, iter->node);
+		n = c_avl_node_prev (iter->node);
 	}
 
 	if (n == NULL)
diff --git a/src/utils_dns.c b/src/utils_dns.c
index a412809..25ef189 100644
--- a/src/utils_dns.c
+++ b/src/utils_dns.c
@@ -429,13 +429,16 @@ static int
 handle_ipv6 (struct ip6_hdr *ipv6, int len)
 {
     char buf[PCAP_SNAPLEN];
-    int offset;
+    unsigned int offset;
     int nexthdr;
 
     struct in6_addr s_addr;
     struct in6_addr d_addr;
     uint16_t payload_len;
 
+    if (0 > len)
+	return (0);
+
     offset = sizeof (struct ip6_hdr);
     nexthdr = ipv6->ip6_nxt;
     s_addr = ipv6->ip6_src;
@@ -459,7 +462,7 @@ handle_ipv6 (struct ip6_hdr *ipv6, int len)
 	uint16_t ext_hdr_len;
 
 	/* Catch broken packets */
-	if ((offset + sizeof (struct ip6_ext)) > len)
+	if ((offset + sizeof (struct ip6_ext)) > (unsigned int)len)
 	    return (0);
 
 	/* Cannot handle fragments. */
@@ -479,7 +482,7 @@ handle_ipv6 (struct ip6_hdr *ipv6, int len)
     } /* while */
 
     /* Catch broken and empty packets */
-    if (((offset + payload_len) > len)
+    if (((offset + payload_len) > (unsigned int)len)
 	    || (payload_len == 0)
 	    || (payload_len > PCAP_SNAPLEN))
 	return (0);
@@ -620,7 +623,7 @@ handle_linux_sll (const u_char *pkt, int len)
     } *hdr;
     uint16_t etype;
 
-    if (len < sizeof (struct sll_header))
+    if ((0 > len) || ((unsigned int)len < sizeof (struct sll_header)))
 	return (0);
 
     hdr  = (struct sll_header *) pkt;
-- 
1.5.4.rc3.37.gfdcf3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://mailman.verplant.org/pipermail/collectd/attachments/20080122/1bf73f0f/attachment.pgp 


More information about the collectd mailing list