[ULOGD2 PATCH 0/4] Misc cleaning

[ULOGD2 PATCH 0/4] Misc cleaning

[Eric Leblond]

Hello,

This small patchset fixes some problems. The main issue is a fix of my previous
patch where I've made a stupid type on a function name.
To avoid this kind of error, I propose an other patch which adds
"-Wall -Wextra" to gcc options.

The main concern here is about IPFIX which is in really bad shape. I've fix
some major error in one of the patch but it remains untested for now and I
don't think it can work properly.

Patchset statistics:
 configure.in                 |    2 ++
 filter/ulogd_filter_IP2BIN.c |    2 +-
 output/ulogd_output_IPFIX.c  |   12 +++++++-----
 util/db.c                    |    2 +-
 4 files changed, 11 insertions(+), 7 deletions(-)

BR,
--
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

Re: [ULOGD2 PATCH 0/4] Misc cleaning

[Pablo Neira Ayuso]

Eric Leblond wrote:
> Hello,
> 
> This small patchset fixes some problems. The main issue is a fix of my previous
> patch where I've made a stupid type on a function name.
> To avoid this kind of error, I propose an other patch which adds
> "-Wall -Wextra" to gcc options.
> 
> The main concern here is about IPFIX which is in really bad shape. I've fix
> some major error in one of the patch but it remains untested for now and I
> don't think it can work properly.

All applied. Thanks. Eric, I'd like to resolve the remaining issues
related with the link layer logging before getting into the IPFIX
problems. Please, have a look at my previous email.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [ULOGD2 PATCH 0/4] Misc cleaning

[Pablo Neira Ayuso]

Eric Leblond wrote:
> The main concern here is about IPFIX which is in really bad shape. I've fix
> some major error in one of the patch but it remains untested for now and I
> don't think it can work properly.

Indeed. It's broken Do you plan to look into this? If not, I'll fix it
myself. Please, let me know since I don't want to duplicate work. Do you
also plan to send me a patch to fix all those warnings?

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [ULOGD2 PATCH 0/4] Misc cleaning

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Eric Leblond wrote:
>> The main concern here is about IPFIX which is in really bad shape. I've fix
>> some major error in one of the patch but it remains untested for now and I
>> don't think it can work properly.
> 
> Indeed. It's broken Do you plan to look into this? If not, I'll fix it
> myself. Please, let me know since I don't want to duplicate work. Do you
> also plan to send me a patch to fix all those warnings?

I think would prefer to enable the warnings that make sense
individually, -Wextra includes lots if crap like "%s expects
type char *, but argument is void *" or signedness warnings
that are compiler-detectable irrelevant 50% of the time.

OTOH a lot of the warnings in the current -git version indicate
that the code really needs more love, like missing return values
etc, so it seems we really want *some* of the new warnings.

Re: [ULOGD2 PATCH 0/4] Misc cleaning

[Eric Leblond]

[-- Attachment #1: Type: text/plain, Size: 1328 bytes --]

Hello,

On Wednesday, 2008 July 23 at 16:43:56 +0200, Patrick McHardy wrote:
> Pablo Neira Ayuso wrote:
>> Eric Leblond wrote:
>>> The main concern here is about IPFIX which is in really bad shape. I've fix
>>> some major error in one of the patch but it remains untested for now and I
>>> don't think it can work properly.
>>
>> Indeed. It's broken Do you plan to look into this?

No, I've got enough work with other modules ;) I will soon send a
complete a patchset about the hardware header issue.

>> If not, I'll fix it
>> myself. Please, let me know since I don't want to duplicate work. Do you
>> also plan to send me a patch to fix all those warnings?

Yes.

> I think would prefer to enable the warnings that make sense
> individually, -Wextra includes lots if crap like "%s expects
> type char *, but argument is void *" or signedness warnings
> that are compiler-detectable irrelevant 50% of the time.
>
> OTOH a lot of the warnings in the current -git version indicate
> that the code really needs more love, like missing return values
> etc, so it seems we really want *some* of the new warnings.

That's right, I plan to do some warning hunting and code cleaning
when I will have finish my current work.

BR,
-- 
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[ULOGD2 PATCH 0/11] Big warning hunting.

[Eric Leblond]

Hello,

As promised, here's the patchset resulting in hunting the cause of gcc warning.

Only a few mistakes and problems were found but the warning hunt was
necessary.
Main issues are fixed in the patch:
 * Cleanly stop SYSLOG module: close log function was not called.
 * Suppress twice defined key: printpkt was defining twice the same key.
 * Fix some initialisation: ulogd.c had some problem with initialisation.

I've added the option "-Wno-unused-parameter" to CFLAGS to avoid some unfixable
warnings.

If we omit IPFIX that I did not touch, the code is warning free.

Pathset statistics:
 configure.in                              |    2 +-
 filter/raw2packet/ulogd_raw2packet_BASE.c |    5 +----
 filter/ulogd_filter_IFINDEX.c             |    2 +-
 filter/ulogd_filter_IP2BIN.c              |    4 ++--
 filter/ulogd_filter_MARK.c                |    4 ++--
 filter/ulogd_filter_PWSNIFF.c             |    4 ++--
 include/libipulog/libipulog.h             |    2 +-
 input/packet/ulogd_inppkt_ULOG.c          |    2 +-
 libipulog/libipulog.c                     |    4 ++--
 libipulog/ulog_test.c                     |    2 +-
 output/mysql/ulogd_output_MYSQL.c         |    4 +---
 output/pgsql/ulogd_output_PGSQL.c         |    5 ++---
 output/ulogd_output_LOGEMU.c              |    2 +-
 output/ulogd_output_NACCT.c               |    8 ++++----
 output/ulogd_output_SYSLOG.c              |    8 ++++++--
 src/conffile.c                            |    4 ++--
 src/ulogd.c                               |   16 +++++++---------
 util/db.c                                 |   16 +++++++---------
 util/printflow.c                          |    8 ++++----
 util/printpkt.c                           |   15 ++++++---------
 20 files changed, 54 insertions(+), 63 deletions(-)

BR,
--
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

[ULOGD2 PATCH 01/11] Fix some simple warnings.

[Eric Leblond]

This patch fixes some gcc warnings:
 * Unused variables
 * Functions with wrong return (or without return)

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 filter/raw2packet/ulogd_raw2packet_BASE.c |    3 ---
 filter/ulogd_filter_IFINDEX.c             |    2 +-
 filter/ulogd_filter_IP2BIN.c              |    4 ++--
 filter/ulogd_filter_PWSNIFF.c             |    1 -
 output/mysql/ulogd_output_MYSQL.c         |    2 --
 output/pgsql/ulogd_output_PGSQL.c         |    1 -
 src/ulogd.c                               |    3 +--
 util/db.c                                 |    5 +----
 util/printpkt.c                           |    2 --
 9 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/filter/raw2packet/ulogd_raw2packet_BASE.c b/filter/raw2packet/ulogd_raw2packet_BASE.c
index dde1cf3..7c6fd26 100644
--- a/filter/raw2packet/ulogd_raw2packet_BASE.c
+++ b/filter/raw2packet/ulogd_raw2packet_BASE.c
@@ -920,9 +920,6 @@ static int _interp_arp(struct ulogd_pluginstance *pi, u_int32_t len)
 
 static int _interp_bridge(struct ulogd_pluginstance *pi, u_int32_t len)
 {
-	struct ulogd_key *ret = pi->output.keys;
-	const struct sk_buff *skb =
-		GET_VALUE(pi->input.keys, INKEY_RAW_PCKT).ptr;
 	const u_int16_t proto =
 		GET_VALUE(pi->input.keys, INKEY_OOB_PROTOCOL).ui16;
 
diff --git a/filter/ulogd_filter_IFINDEX.c b/filter/ulogd_filter_IFINDEX.c
index 6491127..f7571bd 100644
--- a/filter/ulogd_filter_IFINDEX.c
+++ b/filter/ulogd_filter_IFINDEX.c
@@ -84,7 +84,7 @@ static int nlif_read_cb(int fd, unsigned int what, void *param)
 	if (!(what & ULOGD_FD_READ))
 		return 0;
 
-	nlif_catch(nlif_inst);
+	return nlif_catch(nlif_inst);
 }
 
 static int ifindex_start(struct ulogd_pluginstance *upi)
diff --git a/filter/ulogd_filter_IP2BIN.c b/filter/ulogd_filter_IP2BIN.c
index c4b44fa..2d9fc16 100644
--- a/filter/ulogd_filter_IP2BIN.c
+++ b/filter/ulogd_filter_IP2BIN.c
@@ -147,7 +147,7 @@ static char *ip2bin(struct ulogd_key* inp, int index, char family)
 		default:
 			/* TODO handle error */
 			ulogd_log(ULOGD_NOTICE, "Unknown protocol family\n");
-			return;
+			return NULL;
 	}
 
 	buffer = tmp;
@@ -161,7 +161,7 @@ static char *ip2bin(struct ulogd_key* inp, int index, char family)
 				addr8[0], addr8[1], addr8[2], addr8[3]);
 		if (written != 2 * 4) {
 			buffer[0] = 0;
-			return;
+			return NULL;
 		}
 		buffer += written;
 		addr8 += 4;
diff --git a/filter/ulogd_filter_PWSNIFF.c b/filter/ulogd_filter_PWSNIFF.c
index 95d92b5..290e4cb 100644
--- a/filter/ulogd_filter_PWSNIFF.c
+++ b/filter/ulogd_filter_PWSNIFF.c
@@ -60,7 +60,6 @@ static unsigned char *_get_next_blank(unsigned char* begp, unsigned char *endp)
 
 static int interp_pwsniff(struct ulogd_pluginstance *pi)
 {
-	struct ulogd_key *inp = pi->input.keys;
 	struct ulogd_key *ret = pi->output.keys;
 	struct iphdr *iph;
 	void *protoh;
diff --git a/output/mysql/ulogd_output_MYSQL.c b/output/mysql/ulogd_output_MYSQL.c
index fd650bf..562424b 100644
--- a/output/mysql/ulogd_output_MYSQL.c
+++ b/output/mysql/ulogd_output_MYSQL.c
@@ -102,7 +102,6 @@ static int get_columns_mysql(struct ulogd_pluginstance *upi)
 	struct mysql_instance *mi = (struct mysql_instance *) upi->private;
 	MYSQL_RES *result;
 	MYSQL_FIELD *field;
-	struct ulogd_key *f, *f2;
 	int i;
 
 	if (!mi->dbh) {
@@ -144,7 +143,6 @@ static int get_columns_mysql(struct ulogd_pluginstance *upi)
 	for (i = 0; field = mysql_fetch_field(result); i++) {
 		char buf[ULOGD_MAX_KEYLEN+1];
 		char *underscore;
-		int id;
 
 		/* replace all underscores with dots */
 		strncpy(buf, field->name, ULOGD_MAX_KEYLEN);
diff --git a/output/pgsql/ulogd_output_PGSQL.c b/output/pgsql/ulogd_output_PGSQL.c
index 7ece626..b04526a 100644
--- a/output/pgsql/ulogd_output_PGSQL.c
+++ b/output/pgsql/ulogd_output_PGSQL.c
@@ -183,7 +183,6 @@ static int get_columns_pgsql(struct ulogd_pluginstance *upi)
 	for (i = 0; i < PQntuples(pi->pgres); i++) {
 		char buf[ULOGD_MAX_KEYLEN+1];
 		char *underscore;
-		int id;
 
 		/* replace all underscores with dots */
 		strncpy(buf, PQgetvalue(pi->pgres, i, 0), ULOGD_MAX_KEYLEN);
diff --git a/src/ulogd.c b/src/ulogd.c
index 4e36984..33d58ee 100644
--- a/src/ulogd.c
+++ b/src/ulogd.c
@@ -204,7 +204,6 @@ int ulogd_wildcard_inputkeys(struct ulogd_pluginstance *upi)
 
 	/* second pass: copy key names */
 	llist_for_each_entry(pi_cur, &stack->list, list) {
-		struct ulogd_key *cur;
 		int i;
 
 		for (i = 0; i < pi_cur->plugin->output.num_keys; i++)
@@ -1022,7 +1021,7 @@ static struct option opts[] = {
 	{ "help", 0, NULL, 'h' },
 	{ "configfile", 1, NULL, 'c'},
 	{ "uid", 1, NULL, 'u' },
-	{ "info", 1, NULL, 'i'},
+	{ "info", 1, NULL, 'i' },
 	{ 0 }
 };
 
diff --git a/util/db.c b/util/db.c
index 946458a..9b4da8d 100644
--- a/util/db.c
+++ b/util/db.c
@@ -31,6 +31,7 @@
 #include <errno.h>
 #include <netinet/in.h>
 #include <arpa/inet.h>
+#include <time.h>
 
 #include <ulogd/ulogd.h>
 #include <ulogd/db.h>
@@ -62,8 +63,6 @@ static int sql_createstmt(struct ulogd_pluginstance *upi)
 {
 	struct db_instance *mi = (struct db_instance *) upi->private;
 	unsigned int size;
-	char buf[ULOGD_MAX_KEYLEN];
-	char *underscore;
 	int i;
 	char *table = table_ce(upi->config_kset).u.string;
 	char *procedure = procedure_ce(upi->config_kset).u.string;
@@ -245,8 +244,6 @@ static int __interp_db(struct ulogd_pluginstance *upi)
 		}
 		
 		switch (res->type) {
-			char *tmpstr;
-			struct in_addr addr;
 		case ULOGD_RET_INT8:
 			sprintf(di->stmt_ins, "%d,", res->u.value.i8);
 			break;
diff --git a/util/printpkt.c b/util/printpkt.c
index 90fec0e..c64a4cb 100644
--- a/util/printpkt.c
+++ b/util/printpkt.c
@@ -276,7 +276,6 @@ static int printpkt_ipv4(struct ulogd_key *res, char *buf)
 static int printpkt_ipv6(struct ulogd_key *res, char *buf)
 {
 	char *buf_cur = buf;
-	char tmp[INET6_ADDRSTRLEN];
 
 	if (pp_is_valid(res, KEY_IP_SADDR))
 		buf_cur += sprintf(buf_cur, "SRC=%s ",
@@ -353,7 +352,6 @@ int printpkt_arp(struct ulogd_key *res, char *buf)
 	char *buf_cur = buf;
 	u_int16_t code = 0;
 	u_int8_t *mac;
-	char tmp[INET_ADDRSTRLEN];
 
 	if (pp_is_valid(res, KEY_ARP_SPA))
 		buf_cur += sprintf(buf_cur, "SRC=%s ",
-- 
1.5.4.3

[ULOGD2 PATCH 02/11] Cleanly stop SYSLOG module.

[Eric Leblond]

This patch adds a stop function to the module which closes the connection to
the log system.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 output/ulogd_output_SYSLOG.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/output/ulogd_output_SYSLOG.c b/output/ulogd_output_SYSLOG.c
index 8982e7e..8dd93f3 100644
--- a/output/ulogd_output_SYSLOG.c
+++ b/output/ulogd_output_SYSLOG.c
@@ -151,10 +151,13 @@ static int syslog_configure(struct ulogd_pluginstance *pi,
 	return 0;
 }
 
-static void syslog_fini(void)
+static int syslog_fini(struct ulogd_pluginstance *pi)
 {
 	closelog();
+
+	return 0;
 }
+
 static int syslog_start(struct ulogd_pluginstance *pi)
 {
 	openlog("ulogd", LOG_NDELAY|LOG_PID, LOG_DAEMON);
@@ -177,6 +180,7 @@ static struct ulogd_plugin syslog_plugin = {
 	
 	.configure	= &syslog_configure,
 	.start		= &syslog_start,
+	.stop		= &syslog_fini,
 	.interp		= &_output_syslog,
 	.version	= ULOGD_VERSION,
 };
-- 
1.5.4.3

[ULOGD2 PATCH 03/11] Suppress unused parameter in ipulog_read.

[Eric Leblond]

The function ipulog_read had a timeout parameter which was not
used in the code.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 include/libipulog/libipulog.h    |    2 +-
 input/packet/ulogd_inppkt_ULOG.c |    2 +-
 libipulog/libipulog.c            |    2 +-
 libipulog/ulog_test.c            |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/libipulog/libipulog.h b/include/libipulog/libipulog.h
index a049d27..1f6aee9 100644
--- a/include/libipulog/libipulog.h
+++ b/include/libipulog/libipulog.h
@@ -29,7 +29,7 @@ struct ipulog_handle *ipulog_create_handle(u_int32_t gmask, u_int32_t rmem);
 void ipulog_destroy_handle(struct ipulog_handle *h);
 
 ssize_t ipulog_read(struct ipulog_handle *h,
-		    unsigned char *buf, size_t len, int timeout);
+		    unsigned char *buf, size_t len);
 
 ulog_packet_msg_t *ipulog_get_packet(struct ipulog_handle *h,
 				     const unsigned char *buf,
diff --git a/input/packet/ulogd_inppkt_ULOG.c b/input/packet/ulogd_inppkt_ULOG.c
index c00d9bf..3fdb42e 100644
--- a/input/packet/ulogd_inppkt_ULOG.c
+++ b/input/packet/ulogd_inppkt_ULOG.c
@@ -255,7 +255,7 @@ static int ulog_read_cb(int fd, unsigned int what, void *param)
 		return 0;
 
 	while ((len = ipulog_read(u->libulog_h, u->libulog_buf,
-				 upi->config_kset->ces[0].u.value, 1))) {
+				 upi->config_kset->ces[0].u.value))) {
 		if (len <= 0) {
 			if (errno == EAGAIN)
 				break;
diff --git a/libipulog/libipulog.c b/libipulog/libipulog.c
index 8917bcc..6b7ea35 100644
--- a/libipulog/libipulog.c
+++ b/libipulog/libipulog.c
@@ -190,7 +190,7 @@ int ipulog_set_mode()
 
 /* do a BLOCKING read on an ipulog handle */
 ssize_t ipulog_read(struct ipulog_handle *h, unsigned char *buf,
-		    size_t len, int timeout)
+		    size_t len)
 {
 	return ipulog_netlink_recvfrom(h, buf, len);
 }
diff --git a/libipulog/ulog_test.c b/libipulog/ulog_test.c
index c891764..fd9c925 100644
--- a/libipulog/ulog_test.c
+++ b/libipulog/ulog_test.c
@@ -67,7 +67,7 @@ int main(int argc, char *argv[])
 
 	/* loop receiving packets and handling them over to handle_packet */
 	for (i = 0; i < atoi(argv[1]); i++) {
-		len = ipulog_read(h, buf, MYBUFSIZ, 1);
+		len = ipulog_read(h, buf, MYBUFSIZ);
 		if (len <= 0) {
 			ipulog_perror("ulog_test: short read");
 			exit(1);
-- 
1.5.4.3

[ULOGD2 PATCH 04/11] Fix warning related to signed unsigned comparison.

[Eric Leblond]

This patch fixes the warning related to signed and unsigned comparaison.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 filter/raw2packet/ulogd_raw2packet_BASE.c |    2 +-
 filter/ulogd_filter_MARK.c                |    4 ++--
 filter/ulogd_filter_PWSNIFF.c             |    3 ++-
 libipulog/libipulog.c                     |    2 +-
 src/conffile.c                            |    4 ++--
 src/ulogd.c                               |   10 +++++-----
 util/db.c                                 |    4 ++--
 7 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/filter/raw2packet/ulogd_raw2packet_BASE.c b/filter/raw2packet/ulogd_raw2packet_BASE.c
index 7c6fd26..e61d904 100644
--- a/filter/raw2packet/ulogd_raw2packet_BASE.c
+++ b/filter/raw2packet/ulogd_raw2packet_BASE.c
@@ -694,7 +694,7 @@ static int _interp_iphdr(struct ulogd_pluginstance *pi, u_int32_t len)
 		GET_VALUE(pi->input.keys, INKEY_RAW_PCKT).ptr;
 	void *nexthdr = (u_int32_t *)iph + iph->ihl;
 
-	if (len < sizeof(struct iphdr) || len <= iph->ihl * 4)
+	if (len < sizeof(struct iphdr) || len <= (u_int32_t)(iph->ihl * 4))
 		return ULOGD_IRET_OK;
 	len -= iph->ihl * 4;
 
diff --git a/filter/ulogd_filter_MARK.c b/filter/ulogd_filter_MARK.c
index ff31fe5..98f5dae 100644
--- a/filter/ulogd_filter_MARK.c
+++ b/filter/ulogd_filter_MARK.c
@@ -74,14 +74,14 @@ static int interp_mark(struct ulogd_pluginstance *pi)
 	if (pp_is_valid(inp, KEY_CT_MARK)) {
 		if ((GET_VALUE(inp, KEY_CT_MARK).ui32 &
 			pi->config_kset->ces[MARK_MASK].u.value) !=
-			pi->config_kset->ces[MARK_MARK].u.value
+			(u_int32_t) pi->config_kset->ces[MARK_MARK].u.value
 		   ) {
 			return ULOGD_IRET_STOP;
 		}
 	} else if (pp_is_valid(inp, KEY_OOB_MARK)) {
 		if ((GET_VALUE(inp, KEY_OOB_MARK).ui32 &
 			pi->config_kset->ces[MARK_MASK].u.value) !=
-			pi->config_kset->ces[MARK_MARK].u.value
+			(u_int32_t) pi->config_kset->ces[MARK_MARK].u.value
 		   ) {
 			return ULOGD_IRET_STOP;
 		}
diff --git a/filter/ulogd_filter_PWSNIFF.c b/filter/ulogd_filter_PWSNIFF.c
index 290e4cb..3cbafb6 100644
--- a/filter/ulogd_filter_PWSNIFF.c
+++ b/filter/ulogd_filter_PWSNIFF.c
@@ -66,7 +66,8 @@ static int interp_pwsniff(struct ulogd_pluginstance *pi)
 	struct tcphdr *tcph;	
 	unsigned int tcplen;
 	unsigned char  *ptr, *begp, *pw_begp, *endp, *pw_endp;
-	int len, pw_len, i, cont = 0;
+	int len, pw_len, cont = 0;
+	unsigned int i;
 
 	if (!IS_VALID(pi->input.keys[0]))
 		return ULOGD_IRET_STOP;
diff --git a/libipulog/libipulog.c b/libipulog/libipulog.c
index 6b7ea35..2f58ea6 100644
--- a/libipulog/libipulog.c
+++ b/libipulog/libipulog.c
@@ -96,7 +96,7 @@ ipulog_netlink_recvfrom(const struct ipulog_handle *h,
 		return -1;
 	}
 	nlh = (struct nlmsghdr *)buf;
-	if (nlh->nlmsg_flags & MSG_TRUNC || status > len) {
+	if (nlh->nlmsg_flags & MSG_TRUNC || (size_t) status > len) {
 		ipulog_errno = IPULOG_ERR_TRUNC;
 		return -1;
 	}
diff --git a/src/conffile.c b/src/conffile.c
index 72afb8e..0c1a2a4 100644
--- a/src/conffile.c
+++ b/src/conffile.c
@@ -112,7 +112,7 @@ int config_parse_file(const char *section, struct config_keyset *kset)
 	char *args;
 	int err = 0;
 	int found = 0;
-	int i;
+	unsigned int i;
 	char linebuf[LINE_LEN+1];
 	char *line = linebuf;
 
@@ -147,7 +147,7 @@ int config_parse_file(const char *section, struct config_keyset *kset)
 	/* Parse this section until next section */
 	while (fgets(line, LINE_LEN, cfile))
 	{
-		int i;
+		unsigned int i;
 		char wordbuf[LINE_LEN];
 		char *wordend;
 		
diff --git a/src/ulogd.c b/src/ulogd.c
index 33d58ee..429f472 100644
--- a/src/ulogd.c
+++ b/src/ulogd.c
@@ -204,7 +204,7 @@ int ulogd_wildcard_inputkeys(struct ulogd_pluginstance *upi)
 
 	/* second pass: copy key names */
 	llist_for_each_entry(pi_cur, &stack->list, list) {
-		int i;
+		unsigned int i;
 
 		for (i = 0; i < pi_cur->plugin->output.num_keys; i++)
 			upi->input.keys[index++] = pi_cur->output.keys[i];
@@ -280,7 +280,7 @@ char *type_to_string(int type)
 
 void get_plugin_infos(struct ulogd_plugin *me)
 {
-	int i;
+	unsigned int i;
 	printf("Name: %s\n", me->name);
 	if (me->config_kset) {
 		printf("Config options:\n");
@@ -453,7 +453,7 @@ static void ulogd_clean_results(struct ulogd_pluginstance *pi)
 
 	/* iterate through plugin stack */
 	llist_for_each_entry(cur, &pi->stack->list, list) {
-		int i;
+		unsigned int i;
 		
 		/* iterate through input keys of pluginstance */
 		for (i = 0; i < cur->output.num_keys; i++) {
@@ -598,7 +598,7 @@ find_okey_in_stack(char *name,
 	struct ulogd_pluginstance *pi;
 
 	llist_for_each_entry_reverse(pi, &start->list, list) {
-		int i;
+		unsigned int i;
 
 		if ((void *)&pi->list == &stack->list)
 			return NULL;
@@ -666,7 +666,7 @@ create_stack_resolve_keys(struct ulogd_pluginstance_stack *stack)
 			}
 			/* no need to match keys */
 		} else {
-			int j;
+			unsigned int j;
 
 			/* not the last one in the stack */
 			if (!(pi_cur->plugin->input.type &
diff --git a/util/db.c b/util/db.c
index 9b4da8d..8e9dde6 100644
--- a/util/db.c
+++ b/util/db.c
@@ -63,7 +63,7 @@ static int sql_createstmt(struct ulogd_pluginstance *upi)
 {
 	struct db_instance *mi = (struct db_instance *) upi->private;
 	unsigned int size;
-	int i;
+	unsigned int i;
 	char *table = table_ce(upi->config_kset).u.string;
 	char *procedure = procedure_ce(upi->config_kset).u.string;
 
@@ -223,7 +223,7 @@ static int _init_db(struct ulogd_pluginstance *upi)
 static int __interp_db(struct ulogd_pluginstance *upi)
 {
 	struct db_instance *di = (struct db_instance *) &upi->private;
-	int i;
+	unsigned int i;
 
 	di->stmt_ins = di->stmt_val;
 
-- 
1.5.4.3

[ULOGD2 PATCH 05/11] Suppress some warnings in format string usage.

[Eric Leblond]

This patch cast to (char *) some (void *) to avoid a gcc warning in string
format parsing.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 output/ulogd_output_LOGEMU.c |    2 +-
 output/ulogd_output_NACCT.c  |    8 ++++----
 output/ulogd_output_SYSLOG.c |    2 +-
 util/db.c                    |    2 +-
 util/printflow.c             |    8 ++++----
 util/printpkt.c              |   12 ++++++------
 6 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/output/ulogd_output_LOGEMU.c b/output/ulogd_output_LOGEMU.c
index cab3b0c..edee86f 100644
--- a/output/ulogd_output_LOGEMU.c
+++ b/output/ulogd_output_LOGEMU.c
@@ -102,7 +102,7 @@ static int _output_logemu(struct ulogd_pluginstance *upi)
 			*tmp = '\0';
 
 		fprintf(li->of, "%.15s %s %s", timestr, hostname,
-				res[0].u.source->u.value.ptr);
+				(char *) res[0].u.source->u.value.ptr);
 
 		if (upi->config_kset->ces[1].u.value)
 			fflush(li->of);
diff --git a/output/ulogd_output_NACCT.c b/output/ulogd_output_NACCT.c
index 4897959..0ab2be4 100644
--- a/output/ulogd_output_NACCT.c
+++ b/output/ulogd_output_NACCT.c
@@ -130,9 +130,9 @@ nacct_interp(struct ulogd_pluginstance *pi)
 				 "%u\t%u\t%s\t%u\t%s\t%u\t%u\t%u",
 				 GET_VALUE(inp, KEY_FLOW_END).ui32,
 				 GET_VALUE(inp, KEY_IP_PROTO).ui8,
-				 GET_VALUE(inp, KEY_IP_SADDR).ptr,
+				 (char *) GET_VALUE(inp, KEY_IP_SADDR).ptr,
 				 GET_VALUE(inp, KEY_ICMP_TYPE).ui8,
-				 GET_VALUE(inp, KEY_IP_DADDR).ptr,
+				 (char *) GET_VALUE(inp, KEY_IP_DADDR).ptr,
 				 GET_VALUE(inp, KEY_ICMP_CODE).ui8,
 				 GET_VALUE(inp, KEY_RAW_PKTCNT).ui32,
 				 GET_VALUE(inp, KEY_RAW_PKTLEN).ui32);
@@ -141,9 +141,9 @@ nacct_interp(struct ulogd_pluginstance *pi)
 				 "%u\t%u\t%s\t%u\t%s\t%u\t%u\t%u",
 				 GET_VALUE(inp, KEY_FLOW_END).ui32,
 				 GET_VALUE(inp, KEY_IP_PROTO).ui8,
-				 GET_VALUE(inp, KEY_IP_SADDR).ptr,
+				 (char *) GET_VALUE(inp, KEY_IP_SADDR).ptr,
 				 GET_VALUE(inp, KEY_L4_SPORT).ui16,
-				 GET_VALUE(inp, KEY_IP_DADDR).ptr,
+				 (char *) GET_VALUE(inp, KEY_IP_DADDR).ptr,
 				 GET_VALUE(inp, KEY_L4_DPORT).ui16,
 				 GET_VALUE(inp, KEY_RAW_PKTCNT).ui32,
 				 GET_VALUE(inp, KEY_RAW_PKTLEN).ui32);
diff --git a/output/ulogd_output_SYSLOG.c b/output/ulogd_output_SYSLOG.c
index 8dd93f3..1812cb0 100644
--- a/output/ulogd_output_SYSLOG.c
+++ b/output/ulogd_output_SYSLOG.c
@@ -77,7 +77,7 @@ static int _output_syslog(struct ulogd_pluginstance *upi)
 
 	if (res[0].u.source->flags & ULOGD_RETF_VALID)
 		syslog(li->syslog_level | li->syslog_facility, "%s",
-				res[0].u.source->u.value.ptr);
+				(char *) res[0].u.source->u.value.ptr);
 
 	return ULOGD_IRET_OK;
 }
diff --git a/util/db.c b/util/db.c
index 8e9dde6..68dcc31 100644
--- a/util/db.c
+++ b/util/db.c
@@ -284,7 +284,7 @@ static int __interp_db(struct ulogd_pluginstance *upi)
 			sprintf(di->stmt_ins, "',");
 			break;
 		case ULOGD_RET_RAWSTR:
-			sprintf(di->stmt_ins, "%s,", res->u.value.ptr);
+			sprintf(di->stmt_ins, "%s,", (char *) res->u.value.ptr);
 			break;
 		case ULOGD_RET_RAW:
 			ulogd_log(ULOGD_NOTICE,
diff --git a/util/printflow.c b/util/printflow.c
index 73d2969..bb3c2c4 100644
--- a/util/printflow.c
+++ b/util/printflow.c
@@ -164,12 +164,12 @@ int printflow_print(struct ulogd_key *res, char *buf)
 	if (pp_is_valid(res, PRINTFLOW_ORIG_IP_SADDR))
 		buf_cur += sprintf(buf_cur,
 				   "SRC=%s ", 
-				   GET_VALUE(res, PRINTFLOW_ORIG_IP_SADDR).ptr);
+				   (char *) GET_VALUE(res, PRINTFLOW_ORIG_IP_SADDR).ptr);
 
 	if (pp_is_valid(res, PRINTFLOW_ORIG_IP_DADDR))
 		buf_cur += sprintf(buf_cur,
 				   "DST=%s ",
-				   GET_VALUE(res, PRINTFLOW_ORIG_IP_DADDR).ptr);
+				   (char *) GET_VALUE(res, PRINTFLOW_ORIG_IP_DADDR).ptr);
 
 	if (!pp_is_valid(res, PRINTFLOW_ORIG_IP_PROTOCOL))
 		goto orig_out;
@@ -215,12 +215,12 @@ orig_out:
 	if (pp_is_valid(res, PRINTFLOW_REPLY_IP_SADDR))
 		buf_cur += sprintf(buf_cur,
 				   "SRC=%s ",
-				   GET_VALUE(res,PRINTFLOW_REPLY_IP_SADDR).ptr);
+				   (char *) GET_VALUE(res,PRINTFLOW_REPLY_IP_SADDR).ptr);
 
 	if (pp_is_valid(res, PRINTFLOW_REPLY_IP_DADDR))
 		buf_cur += sprintf(buf_cur,
 				   "DST=%s ",
-				   GET_VALUE(res,PRINTFLOW_REPLY_IP_DADDR).ptr);
+				   (char *) GET_VALUE(res,PRINTFLOW_REPLY_IP_DADDR).ptr);
 
 	if (!pp_is_valid(res, PRINTFLOW_REPLY_IP_PROTOCOL))
 		goto reply_out;
diff --git a/util/printpkt.c b/util/printpkt.c
index c64a4cb..5359b8b 100644
--- a/util/printpkt.c
+++ b/util/printpkt.c
@@ -193,11 +193,11 @@ static int printpkt_ipv4(struct ulogd_key *res, char *buf)
 
 	if (pp_is_valid(res, KEY_IP_SADDR))
 		buf_cur += sprintf(buf_cur, "SRC=%s ",
-				   GET_VALUE(res, KEY_IP_SADDR).ptr);
+				   (char *) GET_VALUE(res, KEY_IP_SADDR).ptr);
 
 	if (pp_is_valid(res, KEY_IP_DADDR))
 		buf_cur += sprintf(buf_cur, "DST=%s ",
-				   GET_VALUE(res, KEY_IP_DADDR).ptr);
+				   (char *) GET_VALUE(res, KEY_IP_DADDR).ptr);
 
 	/* FIXME: add pp_is_valid calls to remainder of file */
 	buf_cur += sprintf(buf_cur,"LEN=%u TOS=%02X PREC=0x%02X TTL=%u ID=%u ", 
@@ -279,11 +279,11 @@ static int printpkt_ipv6(struct ulogd_key *res, char *buf)
 
 	if (pp_is_valid(res, KEY_IP_SADDR))
 		buf_cur += sprintf(buf_cur, "SRC=%s ",
-				   GET_VALUE(res, KEY_IP_SADDR).ptr);
+				   (char *) GET_VALUE(res, KEY_IP_SADDR).ptr);
 
 	if (pp_is_valid(res, KEY_IP_DADDR))
 		buf_cur += sprintf(buf_cur, "DST=%s ",
-				   GET_VALUE(res, KEY_IP_DADDR).ptr);
+				   (char *) GET_VALUE(res, KEY_IP_DADDR).ptr);
 
 	if (pp_is_valid(res, KEY_IP6_PAYLOAD_LEN))
 		buf_cur += sprintf(buf_cur, "LEN=%Zu ",
@@ -355,11 +355,11 @@ int printpkt_arp(struct ulogd_key *res, char *buf)
 
 	if (pp_is_valid(res, KEY_ARP_SPA))
 		buf_cur += sprintf(buf_cur, "SRC=%s ",
-				   GET_VALUE(res, KEY_ARP_SPA).ptr);
+				   (char *) GET_VALUE(res, KEY_ARP_SPA).ptr);
 
 	if (pp_is_valid(res, KEY_ARP_TPA))
 		buf_cur += sprintf(buf_cur, "DST=%s ",
-				   GET_VALUE(res, KEY_ARP_TPA).ptr);
+				   (char *) GET_VALUE(res, KEY_ARP_TPA).ptr);
 
 	buf_cur += sprintf(buf_cur, "PROTO=ARP ");
 
-- 
1.5.4.3

[ULOGD2 PATCH 06/11] Fix warning about lack of parenthesis.

[Eric Leblond]

This patch adds parenthesis around an expression to avoid confusion between
order preference of && and || operators.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 output/pgsql/ulogd_output_PGSQL.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/output/pgsql/ulogd_output_PGSQL.c b/output/pgsql/ulogd_output_PGSQL.c
index b04526a..afab2cc 100644
--- a/output/pgsql/ulogd_output_PGSQL.c
+++ b/output/pgsql/ulogd_output_PGSQL.c
@@ -288,8 +288,8 @@ static int execute_pgsql(struct ulogd_pluginstance *upi,
 	struct pgsql_instance *pi = (struct pgsql_instance *) upi->private;
 
 	pi->pgres = PQexec(pi->dbh, stmt);
-	if (!(pi->pgres && (PQresultStatus(pi->pgres) == PGRES_COMMAND_OK)
-		|| (PQresultStatus(pi->pgres) == PGRES_TUPLES_OK))) {
+	if (!(pi->pgres && ((PQresultStatus(pi->pgres) == PGRES_COMMAND_OK)
+		|| (PQresultStatus(pi->pgres) == PGRES_TUPLES_OK)))) {
 		ulogd_log(ULOGD_ERROR, "execute failed (%s)\n",
 			  PQerrorMessage(pi->dbh));
 		return -1;
-- 
1.5.4.3

[ULOGD2 PATCH 07/11] Fix warning about integer format.

[Eric Leblond]

This patch uses PRIu64 and PRId64 macros from inttypes.h to have a correct
definition of 64 bit integer format for 64bits and 32bits arch.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 util/db.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/util/db.c b/util/db.c
index 68dcc31..61c31a3 100644
--- a/util/db.c
+++ b/util/db.c
@@ -32,6 +32,7 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <time.h>
+#include <inttypes.h>
 
 #include <ulogd/ulogd.h>
 #include <ulogd/db.h>
@@ -254,7 +255,7 @@ static int __interp_db(struct ulogd_pluginstance *upi)
 			sprintf(di->stmt_ins, "%d,", res->u.value.i32);
 			break;
 		case ULOGD_RET_INT64:
-			sprintf(di->stmt_ins, "%lld,", res->u.value.i64);
+			sprintf(di->stmt_ins, "%" PRId64 ",", res->u.value.i64);
 			break;
 		case ULOGD_RET_UINT8:
 			sprintf(di->stmt_ins, "%u,", res->u.value.ui8);
@@ -268,7 +269,7 @@ static int __interp_db(struct ulogd_pluginstance *upi)
 			sprintf(di->stmt_ins, "%u,", res->u.value.ui32);
 			break;
 		case ULOGD_RET_UINT64:
-			sprintf(di->stmt_ins, "%llu,", res->u.value.ui64);
+			sprintf(di->stmt_ins, "%" PRIu64 ",", res->u.value.ui64);
 			break;
 		case ULOGD_RET_BOOL:
 			sprintf(di->stmt_ins, "'%d',", res->u.value.b);
-- 
1.5.4.3

[ULOGD2 PATCH 08/11] Suppress twice defined key.

[Eric Leblond]

This patch fixes a multiple definition of the key TCP_URG.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 util/printpkt.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/util/printpkt.c b/util/printpkt.c
index 5359b8b..29c2ea4 100644
--- a/util/printpkt.c
+++ b/util/printpkt.c
@@ -76,7 +76,6 @@ struct ulogd_key printpkt_keys[] = {
 	[KEY_TCP_WINDOW]	= { .name = "tcp.window", },
 	[KEY_TCP_SYN]		= { .name = "tcp.syn", },
 	[KEY_TCP_ACK]		= { .name = "tcp.ack", },
-	[KEY_TCP_URG]		= { .name = "tcp.urg", },
 	[KEY_TCP_PSH]		= { .name = "tcp.psh", },
 	[KEY_TCP_RST]		= { .name = "tcp.rst", },
 	[KEY_TCP_FIN]		= { .name = "tcp.fin", },
-- 
1.5.4.3

[ULOGD2 PATCH 09/11] Fix some initialisation.

[Eric Leblond]

This patch fixes some improper initialization in ulogd.c.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 src/ulogd.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/src/ulogd.c b/src/ulogd.c
index 429f472..e69079d 100644
--- a/src/ulogd.c
+++ b/src/ulogd.c
@@ -97,7 +97,6 @@ static struct config_keyset ulogd_kset = {
 			.key = "logfile",
 			.type = CONFIG_TYPE_CALLBACK,
 			.options = CONFIG_OPT_NONE,
-			.u.string = ULOGD_LOGFILE_DEFAULT,
 			.u.parser = &logfile_open,
 		},
 		{
@@ -1022,7 +1021,7 @@ static struct option opts[] = {
 	{ "configfile", 1, NULL, 'c'},
 	{ "uid", 1, NULL, 'u' },
 	{ "info", 1, NULL, 'i' },
-	{ 0 }
+	{NULL, 0, NULL, 0}
 };
 
 int main(int argc, char* argv[])
-- 
1.5.4.3

[ULOGD2 PATCH 10/11] Make gcc happy.

[Eric Leblond]

Fix gcc warning related to the lack of parenthesis.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 output/mysql/ulogd_output_MYSQL.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/output/mysql/ulogd_output_MYSQL.c b/output/mysql/ulogd_output_MYSQL.c
index 562424b..94c8c9a 100644
--- a/output/mysql/ulogd_output_MYSQL.c
+++ b/output/mysql/ulogd_output_MYSQL.c
@@ -140,7 +140,7 @@ static int get_columns_mysql(struct ulogd_pluginstance *upi)
 	memset(upi->input.keys, 0, sizeof(struct ulogd_key) *
 						upi->input.num_keys);
 
-	for (i = 0; field = mysql_fetch_field(result); i++) {
+	for (i = 0; (field = mysql_fetch_field(result)); i++) {
 		char buf[ULOGD_MAX_KEYLEN+1];
 		char *underscore;
 
-- 
1.5.4.3

[ULOGD2 PATCH 11/11] Add option to CFLAGS to avoid useless warning.

[Eric Leblond]

This patch adds the "-Wno-unused-parameter" option to CFLAGS. This suppress
gcc warning that can not be fixed due to the usage of generic system like
callback where function definition has to be standardized.

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 configure.in |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/configure.in b/configure.in
index 96cc2d0..8889575 100644
--- a/configure.in
+++ b/configure.in
@@ -30,7 +30,7 @@ dnl Checks for library functions.
 AC_FUNC_VPRINTF
 AC_CHECK_FUNCS(socket strerror)
 
-CFLAGS="$CFLAGS -Wall -Wextra"
+CFLAGS="$CFLAGS -Wall -Wextra -Wno-unused-parameter"
 
 dnl Check for the right nfnetlink version
 LIBNFNETLINK_REQUIRED=0.0.39
-- 
1.5.4.3

Re: [ULOGD2 PATCH 11/11] Add option to CFLAGS to avoid useless warning.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> This patch adds the "-Wno-unused-parameter" option to CFLAGS. This suppress
> gcc warning that can not be fixed due to the usage of generic system like
> callback where function definition has to be standardized.
> 
> Signed-off-by: Eric Leblond <eric@inl.fr>
> ---
>  configure.in |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/configure.in b/configure.in
> index 96cc2d0..8889575 100644
> --- a/configure.in
> +++ b/configure.in
> @@ -30,7 +30,7 @@ dnl Checks for library functions.
>  AC_FUNC_VPRINTF
>  AC_CHECK_FUNCS(socket strerror)
>  
> -CFLAGS="$CFLAGS -Wall -Wextra"
> +CFLAGS="$CFLAGS -Wall -Wextra -Wno-unused-parameter"

Applied from 1 to 10. Thanks Eric.

With regards to this patch. This warning vanishes is we remove -Wextra.

We are not using this flag in any other userspace tools and IIRC Patrick
said that -Wextra did not provide very useful warning. Any comment on this?

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [ULOGD2 PATCH 11/11] Add option to CFLAGS to avoid useless warning.

[Patrick McHardy]

Pablo Neira Ayuso wrote:
> Eric Leblond wrote:
>> This patch adds the "-Wno-unused-parameter" option to CFLAGS. This suppress
>> gcc warning that can not be fixed due to the usage of generic system like
>> callback where function definition has to be standardized.
>>
>> Signed-off-by: Eric Leblond <eric@inl.fr>
>> ---
>>  configure.in |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/configure.in b/configure.in
>> index 96cc2d0..8889575 100644
>> --- a/configure.in
>> +++ b/configure.in
>> @@ -30,7 +30,7 @@ dnl Checks for library functions.
>>  AC_FUNC_VPRINTF
>>  AC_CHECK_FUNCS(socket strerror)
>>  
>> -CFLAGS="$CFLAGS -Wall -Wextra"
>> +CFLAGS="$CFLAGS -Wall -Wextra -Wno-unused-parameter"
> 
> Applied from 1 to 10. Thanks Eric.
> 
> With regards to this patch. This warning vanishes is we remove -Wextra.
> 
> We are not using this flag in any other userspace tools and IIRC Patrick
> said that -Wextra did not provide very useful warning. Any comment on this?


It includes some useful warnings, but also some that tend to
produce lots of false positives. We could enable the useful
ones manually, or simply wait and see how much false positives
we'll actually get.

[ULOGD2 PATCH] Use attribute to avoid warning about unused parameter.

[Eric Leblond]

Following live discussion with Pierre Chifflier, it appears that the flag
-Wno-unused-parameter is deprecated in gcc and has to be replaced by an
attribute: __attribute__ ((unused)).

The following patch fixes all warnings related to unused parameters by
correctly settings the attribute in all unused function parameters. This
is mainly the case for callback functions. It uses a macro named UNUSED
which is simply defined as __attribute__((unused)).

Signed-off-by: Eric Leblond <eric@inl.fr>
---
 filter/ulogd_filter_IFINDEX.c     |    6 +++---
 filter/ulogd_filter_MARK.c        |    2 +-
 include/ulogd/linuxlist.h         |    2 +-
 include/ulogd/ulogd.h             |    4 ++++
 input/flow/ulogd_inpflow_NFCT.c   |   12 ++++++------
 input/packet/ulogd_inppkt_NFLOG.c |    4 ++--
 input/packet/ulogd_inppkt_ULOG.c  |    4 ++--
 output/pcap/ulogd_output_PCAP.c   |    2 +-
 output/pgsql/ulogd_output_PGSQL.c |    6 +++---
 output/ulogd_output_LOGEMU.c      |    2 +-
 output/ulogd_output_NACCT.c       |    2 +-
 output/ulogd_output_OPRINT.c      |    2 +-
 output/ulogd_output_SYSLOG.c      |    6 +++---
 util/db.c                         |    4 ++--
 14 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/filter/ulogd_filter_IFINDEX.c b/filter/ulogd_filter_IFINDEX.c
index f7571bd..21eb00f 100644
--- a/filter/ulogd_filter_IFINDEX.c
+++ b/filter/ulogd_filter_IFINDEX.c
@@ -79,7 +79,7 @@ static int interp_ifindex(struct ulogd_pluginstance *pi)
 	return ULOGD_IRET_OK;
 }
 
-static int nlif_read_cb(int fd, unsigned int what, void *param)
+static int nlif_read_cb(UNUSED int fd, unsigned int what, UNUSED void *param)
 {
 	if (!(what & ULOGD_FD_READ))
 		return 0;
@@ -87,7 +87,7 @@ static int nlif_read_cb(int fd, unsigned int what, void *param)
 	return nlif_catch(nlif_inst);
 }
 
-static int ifindex_start(struct ulogd_pluginstance *upi)
+static int ifindex_start(UNUSED struct ulogd_pluginstance *upi)
 {
 	int rc;
 
@@ -120,7 +120,7 @@ out_nlif:
 	return rc;
 }
 
-static int ifindex_fini(struct ulogd_pluginstance *upi)
+static int ifindex_fini(UNUSED struct ulogd_pluginstance *upi)
 {
 	if (--nlif_users == 0) {
 		ulogd_unregister_fd(&nlif_u_fd);
diff --git a/filter/ulogd_filter_MARK.c b/filter/ulogd_filter_MARK.c
index 98f5dae..31f69f8 100644
--- a/filter/ulogd_filter_MARK.c
+++ b/filter/ulogd_filter_MARK.c
@@ -90,7 +90,7 @@ static int interp_mark(struct ulogd_pluginstance *pi)
 }
 
 static int configure(struct ulogd_pluginstance *upi,
-		     struct ulogd_pluginstance_stack *stack)
+		     UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	ulogd_log(ULOGD_DEBUG, "parsing config file section `%s', "
 		  "plugin `%s'\n", upi->id, upi->plugin->name);
diff --git a/include/ulogd/linuxlist.h b/include/ulogd/linuxlist.h
index a89375e..8c26b10 100644
--- a/include/ulogd/linuxlist.h
+++ b/include/ulogd/linuxlist.h
@@ -7,7 +7,7 @@
 #define inline __inline__
 #endif
 
-static inline void prefetch(const void *x) {;}
+static inline void prefetch(__attribute__((unused)) const void *x) {;}
 
 /**
  * container_of - cast a member of a structure out to the containing structure
diff --git a/include/ulogd/ulogd.h b/include/ulogd/ulogd.h
index 776111a..1ca2376 100644
--- a/include/ulogd/ulogd.h
+++ b/include/ulogd/ulogd.h
@@ -266,4 +266,8 @@ int ulogd_select_main(struct timeval *tv);
 #define IPPROTO_UDPLITE 136
 #endif
 
+#ifndef UNUSED
+#define UNUSED __attribute__((unused))
+#endif
+
 #endif /* _ULOGD_H */
diff --git a/input/flow/ulogd_inpflow_NFCT.c b/input/flow/ulogd_inpflow_NFCT.c
index 58bb0e2..e662d19 100644
--- a/input/flow/ulogd_inpflow_NFCT.c
+++ b/input/flow/ulogd_inpflow_NFCT.c
@@ -703,7 +703,7 @@ static int setnlbufsiz(struct ulogd_pluginstance *upi, int size)
 	return 0;
 }
 
-static int read_cb_nfct(int fd, unsigned int what, void *param)
+static int read_cb_nfct(UNUSED int fd, unsigned int what, void *param)
 {
 	struct nfct_pluginstance *cpi = (struct nfct_pluginstance *) param;
 	struct ulogd_pluginstance *upi = container_of(param,
@@ -764,7 +764,7 @@ static int do_purge(void *data1, void *data2)
 	return 0;
 }
 
-static int overrun_handler(enum nf_conntrack_msg_type type,
+static int overrun_handler(UNUSED enum nf_conntrack_msg_type type,
 			   struct nf_conntrack *ct,
 			   void *data)
 {
@@ -785,7 +785,7 @@ static int overrun_handler(enum nf_conntrack_msg_type type,
 	return NFCT_CB_CONTINUE;
 }
 
-static int read_cb_ovh(int fd, unsigned int what, void *param)
+static int read_cb_ovh(UNUSED int fd, unsigned int what, void *param)
 {
 	struct nfct_pluginstance *cpi = (struct nfct_pluginstance *) param;
 	struct ulogd_pluginstance *upi = container_of(param,
@@ -822,7 +822,7 @@ static int get_ctr_zero(struct ulogd_pluginstance *upi)
 	return nfct_query(cpi->cth, NFCT_Q_DUMP_RESET, &family);
 }
 
-static void getctr_timer_cb(struct ulogd_timer *t, void *data)
+static void getctr_timer_cb(UNUSED struct ulogd_timer *t, void *data)
 {
 	struct ulogd_pluginstance *upi = data;
 	struct nfct_pluginstance *cpi =
@@ -833,7 +833,7 @@ static void getctr_timer_cb(struct ulogd_timer *t, void *data)
 }
 
 static int configure_nfct(struct ulogd_pluginstance *upi,
-			  struct ulogd_pluginstance_stack *stack)
+			  UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	struct nfct_pluginstance *cpi =
 			(struct nfct_pluginstance *)upi->private;
@@ -851,7 +851,7 @@ static int configure_nfct(struct ulogd_pluginstance *upi,
 	return 0;
 }
 
-static void overrun_timeout(struct ulogd_timer *a, void *data)
+static void overrun_timeout(UNUSED struct ulogd_timer *a, void *data)
 {
 	int family = AF_UNSPEC;
 	struct ulogd_pluginstance *upi = data;
diff --git a/input/packet/ulogd_inppkt_NFLOG.c b/input/packet/ulogd_inppkt_NFLOG.c
index ed01f04..a2e8625 100644
--- a/input/packet/ulogd_inppkt_NFLOG.c
+++ b/input/packet/ulogd_inppkt_NFLOG.c
@@ -480,7 +480,7 @@ static int nful_read_cb(int fd, unsigned int what, void *param)
 }
 
 /* callback called by libnfnetlink* for every nlmsg */
-static int msg_cb(struct nflog_g_handle *gh, struct nfgenmsg *nfmsg,
+static int msg_cb(UNUSED struct nflog_g_handle *gh, UNUSED struct nfgenmsg *nfmsg,
 		  struct nflog_data *nfa, void *data)
 {
 	struct ulogd_pluginstance *upi = data;
@@ -498,7 +498,7 @@ static int msg_cb(struct nflog_g_handle *gh, struct nfgenmsg *nfmsg,
 }
 
 static int configure(struct ulogd_pluginstance *upi,
-		     struct ulogd_pluginstance_stack *stack)
+		     UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	ulogd_log(ULOGD_DEBUG, "parsing config file section `%s', "
 		  "plugin `%s'\n", upi->id, upi->plugin->name);
diff --git a/input/packet/ulogd_inppkt_ULOG.c b/input/packet/ulogd_inppkt_ULOG.c
index 3fdb42e..b742532 100644
--- a/input/packet/ulogd_inppkt_ULOG.c
+++ b/input/packet/ulogd_inppkt_ULOG.c
@@ -243,7 +243,7 @@ static int interp_packet(struct ulogd_pluginstance *ip, ulog_packet_msg_t *pkt)
 	return 0;
 }
 
-static int ulog_read_cb(int fd, unsigned int what, void *param)
+static int ulog_read_cb(UNUSED int fd, unsigned int what, void *param)
 {
 	struct ulogd_pluginstance *upi = (struct ulogd_pluginstance *)param;
 	struct ulogd_pluginstance *npi = NULL;
@@ -282,7 +282,7 @@ static int ulog_read_cb(int fd, unsigned int what, void *param)
 }
 
 static int configure(struct ulogd_pluginstance *upi,
-		     struct ulogd_pluginstance_stack *stack)
+		     UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	return config_parse_file(upi->id, upi->config_kset);
 }
diff --git a/output/pcap/ulogd_output_PCAP.c b/output/pcap/ulogd_output_PCAP.c
index ce4393c..988f025 100644
--- a/output/pcap/ulogd_output_PCAP.c
+++ b/output/pcap/ulogd_output_PCAP.c
@@ -243,7 +243,7 @@ static void signal_pcap(struct ulogd_pluginstance *upi, int signal)
 }
 
 static int configure_pcap(struct ulogd_pluginstance *upi,
-			  struct ulogd_pluginstance_stack *stack)
+			  UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	return config_parse_file(upi->id, upi->config_kset);
 }
diff --git a/output/pgsql/ulogd_output_PGSQL.c b/output/pgsql/ulogd_output_PGSQL.c
index afab2cc..a8c201d 100644
--- a/output/pgsql/ulogd_output_PGSQL.c
+++ b/output/pgsql/ulogd_output_PGSQL.c
@@ -276,14 +276,14 @@ static int open_db_pgsql(struct ulogd_pluginstance *upi)
 	return 0;
 }
 
-static int escape_string_pgsql(struct ulogd_pluginstance *upi,
-			       char *dst, const char *src, unsigned int len)
+static int escape_string_pgsql(UNUSED struct ulogd_pluginstance *upi,
+			       char *dst, const char *src, UNUSED unsigned int len)
 {
 	return PQescapeString(dst, src, strlen(src)); 
 }
 
 static int execute_pgsql(struct ulogd_pluginstance *upi,
-			 const char *stmt, unsigned int len)
+			 const char *stmt, UNUSED unsigned int len)
 {
 	struct pgsql_instance *pi = (struct pgsql_instance *) upi->private;
 
diff --git a/output/ulogd_output_LOGEMU.c b/output/ulogd_output_LOGEMU.c
index edee86f..1604d83 100644
--- a/output/ulogd_output_LOGEMU.c
+++ b/output/ulogd_output_LOGEMU.c
@@ -177,7 +177,7 @@ static int fini_logemu(struct ulogd_pluginstance *pi) {
 }
 
 static int configure_logemu(struct ulogd_pluginstance *pi,
-			    struct ulogd_pluginstance_stack *stack)
+			    UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	/* FIXME: error handling */
 	ulogd_log(ULOGD_DEBUG, "parsing config file section %s\n", pi->id);
diff --git a/output/ulogd_output_NACCT.c b/output/ulogd_output_NACCT.c
index 0ab2be4..8fce403 100644
--- a/output/ulogd_output_NACCT.c
+++ b/output/ulogd_output_NACCT.c
@@ -199,7 +199,7 @@ sighup_handler_print(struct ulogd_pluginstance *pi, int signal)
 
 static int
 nacct_conf(struct ulogd_pluginstance *pi,
-		   struct ulogd_pluginstance_stack *stack)
+		   UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	int ret;
 
diff --git a/output/ulogd_output_OPRINT.c b/output/ulogd_output_OPRINT.c
index c2fce38..705694c 100644
--- a/output/ulogd_output_OPRINT.c
+++ b/output/ulogd_output_OPRINT.c
@@ -138,7 +138,7 @@ static void sighup_handler_print(struct ulogd_pluginstance *upi, int signal)
 }
 
 static int oprint_configure(struct ulogd_pluginstance *upi,
-			    struct ulogd_pluginstance_stack *stack)
+			    UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	int ret;
 
diff --git a/output/ulogd_output_SYSLOG.c b/output/ulogd_output_SYSLOG.c
index 1812cb0..3098c78 100644
--- a/output/ulogd_output_SYSLOG.c
+++ b/output/ulogd_output_SYSLOG.c
@@ -83,7 +83,7 @@ static int _output_syslog(struct ulogd_pluginstance *upi)
 }
 		
 static int syslog_configure(struct ulogd_pluginstance *pi,
-			    struct ulogd_pluginstance_stack *stack)
+			    UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	int syslog_facility, syslog_level;
 	char *facility, *level;
@@ -151,14 +151,14 @@ static int syslog_configure(struct ulogd_pluginstance *pi,
 	return 0;
 }
 
-static int syslog_fini(struct ulogd_pluginstance *pi)
+static int syslog_fini(UNUSED struct ulogd_pluginstance *pi)
 {
 	closelog();
 
 	return 0;
 }
 
-static int syslog_start(struct ulogd_pluginstance *pi)
+static int syslog_start(UNUSED struct ulogd_pluginstance *pi)
 {
 	openlog("ulogd", LOG_NDELAY|LOG_PID, LOG_DAEMON);
 
diff --git a/util/db.c b/util/db.c
index 61c31a3..dea0adb 100644
--- a/util/db.c
+++ b/util/db.c
@@ -51,7 +51,7 @@ int ulogd_db_interp(struct ulogd_pluginstance *upi)
 }
 
 /* no connection, plugin disabled */
-static int disabled_interp_db(struct ulogd_pluginstance *upi)
+static int disabled_interp_db(UNUSED struct ulogd_pluginstance *upi)
 {
 	return 0;
 }
@@ -103,7 +103,7 @@ static int sql_createstmt(struct ulogd_pluginstance *upi)
 static int _init_db(struct ulogd_pluginstance *upi);
 
 int ulogd_db_configure(struct ulogd_pluginstance *upi,
-			struct ulogd_pluginstance_stack *stack)
+			UNUSED struct ulogd_pluginstance_stack *stack)
 {
 	struct db_instance *di = (struct db_instance *) upi->private;
 	int ret;
-- 
1.5.6.2

Re: [ULOGD2 PATCH] Use attribute to avoid warning about unused parameter.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> Following live discussion with Pierre Chifflier, it appears that the flag
> -Wno-unused-parameter is deprecated in gcc and has to be replaced by an
> attribute: __attribute__ ((unused)).
> 
> The following patch fixes all warnings related to unused parameters by
> correctly settings the attribute in all unused function parameters. This
> is mainly the case for callback functions. It uses a macro named UNUSED
> which is simply defined as __attribute__((unused)).

I don't like polluting the code with tons of UNUSED flags, sorry. I
prefer if we use several -Wxyz options that may be report useful
warnings instead of -Wextra.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers

Re: [ULOGD2 PATCH] Use attribute to avoid warning about unused parameter.

[Eric Leblond]

Hi,

On Thursday, 2008 July 31 at 16:46:01 +0200, Pablo Neira Ayuso wrote:
> Eric Leblond wrote:
> > Following live discussion with Pierre Chifflier, it appears that the flag
> > -Wno-unused-parameter is deprecated in gcc and has to be replaced by an
> > attribute: __attribute__ ((unused)).
> > 
> > The following patch fixes all warnings related to unused parameters by
> > correctly settings the attribute in all unused function parameters. This
> > is mainly the case for callback functions. It uses a macro named UNUSED
> > which is simply defined as __attribute__((unused)).
> 
> I don't like polluting the code with tons of UNUSED flags, sorry.

I don't like that too.

> I prefer if we use several -Wxyz options that may be report useful
> warnings instead of -Wextra.

But, the fact is that globally switching the flag can be a way to
miss some really useful warning. Patch 03/11 of this patchset
was done after seing a gcc warning.

BR,
-- 
Eric Leblond
INL: http://www.inl.fr/
NuFW: http://www.nufw.org/

Re: [ULOGD2 PATCH 11/11] Add option to CFLAGS to avoid useless warning.

[Pablo Neira Ayuso]

Eric Leblond wrote:
> This patch adds the "-Wno-unused-parameter" option to CFLAGS. This suppress
> gcc warning that can not be fixed due to the usage of generic system like
> callback where function definition has to be standardized.

I have applied this, at least until we find a better way to get the
useful warnings that -Wextra reports.

-- 
"Los honestos son inadaptados sociales" -- Les Luthiers