Re: [ulogd2 PATCH v4 00/32] Fixes for compiler warnings

[Jeremy Sowden <jeremy@azazel.net>]

[-- Attachment #1.1: Type: text/plain, Size: 2889 bytes --]

On 2022-01-03, at 19:10:13 +0100, Pablo Neira Ayuso wrote:
> On Mon, Dec 06, 2021 at 11:21:01PM +0100, Pablo Neira Ayuso wrote:
> > On Tue, Nov 30, 2021 at 10:55:28AM +0000, Jeremy Sowden wrote:
> > > This patch-set fixes all the warnings reported by gcc 11.
> > >
> > > Most of the warnings concern fall-throughs in switches, possibly
> > > problematic uses of functions like `strncpy` and `strncat` and
> > > possible truncation of output by `sprintf` and its siblings.
> > >
> > > Some of the patches fix bugs revealed by warnings, some tweak code
> > > to avoid warnings, others fix or improve things I noticed while
> > > looking at the warnings.
> > >
> > > Changes since v3:
> > >
> > >   * When publishing v3 I accidentally sent out two different
> > >     versions of the patch-set under one cover-letter.  There are
> > >     no code-changes in v4: it just omits the earlier superseded
> > >     patches.
> >
> > Applied from 1 to 19 (all inclusive)
>
> Applied remaining patches with comments.
>
> - Patch #20, #24 maybe consider conversion to snprintf at some point,
>   not your fault, this code is using sprintf in many spots. I think
>   the only problematic scenario which might trigger problems is the
>   configuration path using too long object names.

Yeah, there definitely were other places where I started to make
changes, but held off to stop the patch-set becoming even bigger than it
already was and focus on fixing the warnings.

> - Patch #21, #22 and #25, maybe consolidate this database field from
>   _ to . in a common function.

I've got the beginnings of a patch-set to do some tidying of the DB API.

There's an unused local variable left in the SQLITE3 plug-in.  I've
attached a patch to remove it.

> - Patch #27, tm_gmtoff mod 86400 is really required? tm_gmtoff can be
>   either -12/+12 * 60 * 60, simple assignment to integer should calm
>   down the compiler?

The compiler wasn't smart enough to know that the range of tm_gmtoff is
±43200, so it couldn't work out that the hours would fit in `%+03d`
without the mod.

> - Patch #80, I guess you picked 80 just to provide a sufficiently
>   large buffer to calm down compiler.

Correct.

> - Patch #31: I have replaced this patch with a check from .start and
>   .signal paths to validate the unix socket path. The signal path of
>   ulogd2 is problematic since configuration file errors should
>   likely stop the daemon. I'll post it after this email.

Looks good.

> - Patch #32: this IPFIX plugin was tested with wireshark according to
>   4f639231c83b ("IPFIX: Add IPFIX output plugin"), I wonder if this
>   attribute((packed)) is breaking anything, or maybe this was all
>   tested on 32-bit?

I'll take a look.

> Anyway, after this update it's probably better to look at using
> pkg-config in the build system.

J.

[-- Attachment #1.2: 0001-output-SQLITE3-remove-unused-variable.patch --]
[-- Type: text/x-diff, Size: 943 bytes --]

From e45879a7ea5529c26f369c297295332143ee8420 Mon Sep 17 00:00:00 2001
From: Jeremy Sowden <jeremy@azazel.net>
Date: Wed, 5 Jan 2022 22:37:21 +0000
Subject: [PATCH] output: SQLITE3: remove unused variable

There's local variable left over from a previous tidy-up.  Remove it.

Fixes: 67b0be90f16f ("output: SQLITE3: improve mapping of fields to DB columns")
Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 output/sqlite3/ulogd_output_SQLITE3.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/output/sqlite3/ulogd_output_SQLITE3.c b/output/sqlite3/ulogd_output_SQLITE3.c
index 51eab782cc9d..0a9ad67edcff 100644
--- a/output/sqlite3/ulogd_output_SQLITE3.c
+++ b/output/sqlite3/ulogd_output_SQLITE3.c
@@ -320,7 +320,6 @@ sqlite3_init_db(struct ulogd_pluginstance *pi)
 	}
 
 	for (col = 0; col < num_cols; col++) {
-		char *underscore;
 		struct field *f;
 
 		/* prepend it to the linked list */
-- 
2.34.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]