[PATCH conntrack-tools 0/4] Fixes for yacc parser compilation warnings

[PATCH conntrack-tools 0/4] Fixes for yacc parser compilation warnings

[Jeremy Sowden]

Certain classes of compiler warnings have in the past been disabled for
the sources generated by yacc and flex because "yacc and lex generate
dirty code", to quote the comment in the Makefile.  However, even if
this may have been the case at one time, currently none of the warnings
being suppressed relate to generated code.

The number of disabled classes was reduced last year:

  https://lore.kernel.org/netfilter-devel/20221124100804.25674-1-pablo@netfilter.org/#r

This patch-set fixes the remaining warnings.

* Patch 1 improves the formatting of src/Makefile.am a bit.
* Patch 2 removes the `-Wno-*` flags.
* Patches 3 & 4 fix the resulting warnings.

Jeremy Sowden (4):
  build: reformat and sort `conntrack_LDADD` and `conntrackd_SOURCES`
  build: stop suppressing warnings for generated sources
  read_config_yy: correct `yyerror` prototype
  read_config_yy: correct arguments passed to `inet_aton`

 src/Makefile.am      | 80 +++++++++++++++++++++++++++++++++-----------
 src/read_config_yy.y | 30 ++++++++++++-----
 2 files changed, 81 insertions(+), 29 deletions(-)

-- 
2.40.1

[PATCH conntrack-tools 1/4] build: reformat and sort `conntrack_LDADD` and `conntrackd_SOURCES`

[Jeremy Sowden]

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/Makefile.am | 77 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 17 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 2986ab3b4d4f..4ea573abc12d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -11,7 +11,18 @@ BUILT_SOURCES = read_config_yy.h
 sbin_PROGRAMS = conntrack conntrackd nfct
 
 conntrack_SOURCES = conntrack.c
-conntrack_LDADD = ../extensions/libct_proto_tcp.la ../extensions/libct_proto_udp.la ../extensions/libct_proto_udplite.la ../extensions/libct_proto_icmp.la ../extensions/libct_proto_icmpv6.la ../extensions/libct_proto_sctp.la ../extensions/libct_proto_dccp.la ../extensions/libct_proto_gre.la ../extensions/libct_proto_unknown.la ${LIBNETFILTER_CONNTRACK_LIBS} ${LIBMNL_LIBS} ${LIBNFNETLINK_LIBS}
+conntrack_LDADD   = ../extensions/libct_proto_dccp.la    \
+                    ../extensions/libct_proto_gre.la     \
+                    ../extensions/libct_proto_icmp.la    \
+                    ../extensions/libct_proto_icmpv6.la  \
+                    ../extensions/libct_proto_sctp.la    \
+                    ../extensions/libct_proto_tcp.la     \
+                    ../extensions/libct_proto_udp.la     \
+                    ../extensions/libct_proto_udplite.la \
+                    ../extensions/libct_proto_unknown.la \
+                    ${LIBMNL_LIBS}                       \
+                    ${LIBNETFILTER_CONNTRACK_LIBS}       \
+                    ${LIBNFNETLINK_LIBS}
 
 nfct_SOURCES = nfct.c
 
@@ -35,22 +46,54 @@ if HAVE_CTHELPER
 nfct_LDADD += ${LIBNETFILTER_CTHELPER_LIBS}
 endif
 
-conntrackd_SOURCES = alarm.c main.c run.c hash.c queue.c queue_tx.c rbtree.c \
-		    local.c log.c mcast.c udp.c netlink.c vector.c \
-		    filter.c fds.c event.c process.c origin.c date.c \
-		    cache.c cache-ct.c cache-exp.c \
-		    cache_timer.c \
-		    ctnl.c \
-		    sync-mode.c sync-alarm.c sync-ftfw.c sync-notrack.c \
-		    traffic_stats.c stats-mode.c \
-		    network.c cidr.c \
-		    build.c parse.c \
-		    channel.c multichannel.c channel_mcast.c channel_udp.c \
-		    tcp.c channel_tcp.c \
-		    external_cache.c external_inject.c \
-		    internal_cache.c internal_bypass.c \
-		    read_config_yy.y read_config_lex.l \
-		    stack.c resync.c
+conntrackd_SOURCES = alarm.c            \
+                     build.c            \
+                     cache.c            \
+                     cache-ct.c         \
+                     cache-exp.c        \
+                     cache_timer.c      \
+                     channel.c          \
+                     channel_mcast.c    \
+                     channel_tcp.c      \
+                     channel_udp.c      \
+                     cidr.c             \
+                     ctnl.c             \
+                     date.c             \
+                     event.c            \
+                     external_cache.c   \
+                     external_inject.c  \
+                     fds.c              \
+                     filter.c           \
+                     hash.c             \
+                     internal_bypass.c  \
+                     internal_cache.c   \
+                     local.c            \
+                     log.c              \
+                     main.c             \
+                     mcast.c            \
+                     multichannel.c     \
+                     netlink.c          \
+                     network.c          \
+                     origin.c           \
+                     parse.c            \
+                     process.c          \
+                     queue.c            \
+                     queue_tx.c         \
+                     rbtree.c           \
+                     read_config_lex.l  \
+                     read_config_yy.y   \
+                     resync.c           \
+                     run.c              \
+                     stack.c            \
+                     stats-mode.c       \
+                     sync-alarm.c       \
+                     sync-ftfw.c        \
+                     sync-mode.c        \
+                     sync-notrack.c     \
+                     tcp.c              \
+                     traffic_stats.c    \
+                     udp.c              \
+                     vector.c
 
 if HAVE_CTHELPER
 conntrackd_SOURCES += cthelper.c helpers.c utils.c expect.c
-- 
2.40.1

[PATCH conntrack-tools 2/4] build: stop suppressing warnings for generated sources

[Jeremy Sowden]

Contrary to the comment that yacc and lex generate dirty code, none of
the warnings being suppressed are in the generated code.  Stop suppressing
them in order to fix the code.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/Makefile.am | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/src/Makefile.am b/src/Makefile.am
index 4ea573abc12d..352aa37c9fa4 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -103,9 +103,6 @@ if HAVE_SYSTEMD
 conntrackd_SOURCES += systemd.c
 endif
 
-# yacc and lex generate dirty code
-read_config_yy.o read_config_lex.o: AM_CFLAGS += -Wno-incompatible-pointer-types -Wno-discarded-qualifiers
-
 conntrackd_LDADD = ${LIBMNL_LIBS} ${LIBNETFILTER_CONNTRACK_LIBS} \
 		   ${libdl_LIBS} ${LIBNFNETLINK_LIBS}
 
-- 
2.40.1

[PATCH conntrack-tools 3/4] read_config_yy: correct `yyerror` prototype

[Jeremy Sowden]

Change it to take a `const char *`.  It doesn't modify the string and yacc
passes string literals, so cause compiler warnings.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/read_config_yy.y | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/read_config_yy.y b/src/read_config_yy.y
index f06c6afff7cb..be927c049056 100644
--- a/src/read_config_yy.y
+++ b/src/read_config_yy.y
@@ -47,7 +47,7 @@ extern char *yytext;
 extern int   yylineno;
 
 int yylex (void);
-int yyerror (char *msg);
+int yyerror (const char *msg);
 void yyrestart (FILE *input_file);
 
 struct ct_conf conf;
@@ -1681,7 +1681,7 @@ helper_policy_expect_timeout: T_HELPER_EXPECT_TIMEOUT T_NUMBER
 %%
 
 int __attribute__((noreturn))
-yyerror(char *msg)
+yyerror(const char *msg)
 {
 	dlog(LOG_ERR, "parsing config file in line (%d), symbol '%s': %s",
 	     yylineno, yytext, msg);
-- 
2.40.1

[PATCH conntrack-tools 4/4] read_config_yy: correct arguments passed to `inet_aton`

[Jeremy Sowden]

`inet_aton` expects a `struct in_addr *`.  In a number of calls, we pass
pointers to structs or unions which contain a `struct in_addr` member.  Pass
pointers to the members instead.  In another call, we pass a pointer to a
uint32_t.  Cast it.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/read_config_yy.y | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/read_config_yy.y b/src/read_config_yy.y
index be927c049056..d9b02ab37c5a 100644
--- a/src/read_config_yy.y
+++ b/src/read_config_yy.y
@@ -246,9 +246,11 @@ multicast_options :
 
 multicast_option : T_IPV4_ADDR T_IP
 {
+	struct channel_conf *channel_conf = &conf.channel[conf.channel_num];
+
 	__max_dedicated_links_reached();
 
-	if (!inet_aton($2, &conf.channel[conf.channel_num].u.mcast.in)) {
+	if (!inet_aton($2, &channel_conf->u.mcast.in.inet_addr)) {
 		dlog(LOG_WARNING, "%s is not a valid IPv4 address", $2);
 		free($2);
 		break;
@@ -310,9 +312,11 @@ multicast_option : T_IPV6_ADDR T_IP
 
 multicast_option : T_IPV4_IFACE T_IP
 {
+	struct channel_conf *channel_conf = &conf.channel[conf.channel_num];
+
 	__max_dedicated_links_reached();
 
-	if (!inet_aton($2, &conf.channel[conf.channel_num].u.mcast.ifa)) {
+	if (!inet_aton($2, &channel_conf->u.mcast.ifa.interface_addr)) {
 		dlog(LOG_WARNING, "%s is not a valid IPv4 address", $2);
 		free($2);
 		break;
@@ -423,9 +427,11 @@ udp_options :
 
 udp_option : T_IPV4_ADDR T_IP
 {
+	struct channel_conf *channel_conf = &conf.channel[conf.channel_num];
+
 	__max_dedicated_links_reached();
 
-	if (!inet_aton($2, &conf.channel[conf.channel_num].u.udp.server.ipv4)) {
+	if (!inet_aton($2, &channel_conf->u.udp.server.ipv4.inet_addr)) {
 		dlog(LOG_WARNING, "%s is not a valid IPv4 address", $2);
 		free($2);
 		break;
@@ -456,9 +462,11 @@ udp_option : T_IPV6_ADDR T_IP
 
 udp_option : T_IPV4_DEST_ADDR T_IP
 {
+	struct channel_conf *channel_conf = &conf.channel[conf.channel_num];
+
 	__max_dedicated_links_reached();
 
-	if (!inet_aton($2, &conf.channel[conf.channel_num].u.udp.client)) {
+	if (!inet_aton($2, &channel_conf->u.udp.client.inet_addr)) {
 		dlog(LOG_WARNING, "%s is not a valid IPv4 address", $2);
 		free($2);
 		break;
@@ -574,9 +582,11 @@ tcp_options :
 
 tcp_option : T_IPV4_ADDR T_IP
 {
+	struct channel_conf *channel_conf = &conf.channel[conf.channel_num];
+
 	__max_dedicated_links_reached();
 
-	if (!inet_aton($2, &conf.channel[conf.channel_num].u.tcp.server.ipv4)) {
+	if (!inet_aton($2, &channel_conf->u.tcp.server.ipv4.inet_addr)) {
 		dlog(LOG_WARNING, "%s is not a valid IPv4 address", $2);
 		free($2);
 		break;
@@ -607,9 +617,11 @@ tcp_option : T_IPV6_ADDR T_IP
 
 tcp_option : T_IPV4_DEST_ADDR T_IP
 {
+	struct channel_conf *channel_conf = &conf.channel[conf.channel_num];
+
 	__max_dedicated_links_reached();
 
-	if (!inet_aton($2, &conf.channel[conf.channel_num].u.tcp.client)) {
+	if (!inet_aton($2, &channel_conf->u.tcp.client.inet_addr)) {
 		dlog(LOG_WARNING, "%s is not a valid IPv4 address", $2);
 		free($2);
 		break;
@@ -1239,7 +1251,7 @@ filter_address_item : T_IPV4_ADDR T_IP
 		}
 	}
 
-	if (!inet_aton($2, &ip.ipv4)) {
+	if (!inet_aton($2, (struct in_addr *) &ip.ipv4)) {
 		dlog(LOG_WARNING, "%s is not a valid IPv4, ignoring", $2);
 		free($2);
 		break;
-- 
2.40.1

Re: [PATCH conntrack-tools 0/4] Fixes for yacc parser compilation warnings

[Pablo Neira Ayuso]

On Sat, Aug 26, 2023 at 05:32:22PM +0100, Jeremy Sowden wrote:
> Certain classes of compiler warnings have in the past been disabled for
> the sources generated by yacc and flex because "yacc and lex generate
> dirty code", to quote the comment in the Makefile.  However, even if
> this may have been the case at one time, currently none of the warnings
> being suppressed relate to generated code.
> 
> The number of disabled classes was reduced last year:
> 
>   https://lore.kernel.org/netfilter-devel/20221124100804.25674-1-pablo@netfilter.org/#r
> 
> This patch-set fixes the remaining warnings.
> 
> * Patch 1 improves the formatting of src/Makefile.am a bit.
> * Patch 2 removes the `-Wno-*` flags.
> * Patches 3 & 4 fix the resulting warnings.

Series applied, thanks