public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 5/9] clang-tidy: Enable the bugprone-macro-parentheses check
Date: Thu,  6 Jun 2024 20:09:45 +1000	[thread overview]
Message-ID: <20240606100949.1250958-6-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240606100949.1250958-1-david@gibson.dropbear.id.au>

We globally disabled this, with a justification lumped together with
several checks about braces.  They don't really go together, the others
are essentially a stylistic choice which doesn't match our style.  Omitting
brackets on macro parameters can lead to real and hard to track down bugs
if an expression is ever passed to the macro instead of a plain identifier.

We've only gotten away with the macros which trigger the warning, because
of other conventions its been unlikely to invoke them with anything other
than a simple identifier.  Fix the macros, and enable the warning for the
future.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 Makefile     |  2 --
 tap.c        | 37 +++++++++++++++++++------------------
 tcp.c        |  8 ++++----
 tcp_splice.c |  4 ++--
 4 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/Makefile b/Makefile
index 22f05813..4d00f48c 100644
--- a/Makefile
+++ b/Makefile
@@ -192,7 +192,6 @@ docs: README.md
 # - llvmlibc-restrict-system-libc-headers
 #	TODO: this is Linux-only for the moment, nice to fix eventually
 #
-# - bugprone-macro-parentheses
 # - google-readability-braces-around-statements
 # - hicpp-braces-around-statements
 # - readability-braces-around-statements
@@ -269,7 +268,6 @@ clang-tidy: $(SRCS) $(HEADERS)
 	-clang-analyzer-valist.Uninitialized,\
 	-cppcoreguidelines-init-variables,\
 	-bugprone-assignment-in-if-condition,\
-	-bugprone-macro-parentheses,\
 	-google-readability-braces-around-statements,\
 	-hicpp-braces-around-statements,\
 	-readability-braces-around-statements,\
diff --git a/tap.c b/tap.c
index 2ea08491..26084b48 100644
--- a/tap.c
+++ b/tap.c
@@ -674,18 +674,18 @@ resume:
 			continue;
 		}
 
-#define L4_MATCH(iph, uh, seq)						\
-	(seq->protocol == iph->protocol &&				\
-	 seq->source   == uh->source    && seq->dest  == uh->dest &&	\
-	 seq->saddr.s_addr == iph->saddr && seq->daddr.s_addr == iph->daddr)
+#define L4_MATCH(iph, uh, seq)							\
+	((seq)->protocol == (iph)->protocol &&					\
+	 (seq)->source   == (uh)->source    && (seq)->dest  == (uh)->dest &&	\
+	 (seq)->saddr.s_addr == (iph)->saddr && (seq)->daddr.s_addr == (iph)->daddr)
 
 #define L4_SET(iph, uh, seq)						\
 	do {								\
-		seq->protocol		= iph->protocol;		\
-		seq->source		= uh->source;			\
-		seq->dest		= uh->dest;			\
-		seq->saddr.s_addr	= iph->saddr;			\
-		seq->daddr.s_addr	= iph->daddr;			\
+		(seq)->protocol		= (iph)->protocol;		\
+		(seq)->source		= (uh)->source;			\
+		(seq)->dest		= (uh)->dest;			\
+		(seq)->saddr.s_addr	= (iph)->saddr;			\
+		(seq)->daddr.s_addr	= (iph)->daddr;			\
 	} while (0)
 
 		if (seq && L4_MATCH(iph, uh, seq) && seq->p.count < UIO_MAXIOV)
@@ -848,18 +848,19 @@ resume:
 		}
 
 #define L4_MATCH(ip6h, proto, uh, seq)					\
-	(seq->protocol == proto         &&				\
-	 seq->source   == uh->source    && seq->dest  == uh->dest &&	\
-	 IN6_ARE_ADDR_EQUAL(&seq->saddr, saddr)			  &&	\
-	 IN6_ARE_ADDR_EQUAL(&seq->daddr, daddr))
+		((seq)->protocol == (proto)                &&		\
+		 (seq)->source   == (uh)->source           &&		\
+		 (seq)->dest == (uh)->dest                 &&		\
+		 IN6_ARE_ADDR_EQUAL(&(seq)->saddr, saddr)  &&		\
+		 IN6_ARE_ADDR_EQUAL(&(seq)->daddr, daddr))
 
 #define L4_SET(ip6h, proto, uh, seq)					\
 	do {								\
-		seq->protocol	= proto;				\
-		seq->source	= uh->source;				\
-		seq->dest	= uh->dest;				\
-		seq->saddr	= *saddr;				\
-		seq->daddr	= *daddr;				\
+		(seq)->protocol	= (proto);				\
+		(seq)->source	= (uh)->source;				\
+		(seq)->dest	= (uh)->dest;				\
+		(seq)->saddr	= *saddr;				\
+		(seq)->daddr	= *daddr;				\
 	} while (0)
 
 		if (seq && L4_MATCH(ip6h, proto, uh, seq) &&
diff --git a/tcp.c b/tcp.c
index ff1198dd..0dccde9c 100644
--- a/tcp.c
+++ b/tcp.c
@@ -326,7 +326,7 @@
 
 #define WINDOW_DEFAULT			14600		/* RFC 6928 */
 #ifdef HAS_SND_WND
-# define KERNEL_REPORTS_SND_WND(c)	(c->tcp.kernel_snd_wnd)
+# define KERNEL_REPORTS_SND_WND(c)	((c)->tcp.kernel_snd_wnd)
 #else
 # define KERNEL_REPORTS_SND_WND(c)	(0 && (c))
 #endif
@@ -373,9 +373,9 @@
 #define CONN_V4(conn)		(!!inany_v4(&(conn)->faddr))
 #define CONN_V6(conn)		(!CONN_V4(conn))
 #define CONN_IS_CLOSING(conn)						\
-	((conn->events & ESTABLISHED) &&				\
-	 (conn->events & (SOCK_FIN_RCVD | TAP_FIN_RCVD)))
-#define CONN_HAS(conn, set)	((conn->events & (set)) == (set))
+	(((conn)->events & ESTABLISHED) &&				\
+	 ((conn)->events & (SOCK_FIN_RCVD | TAP_FIN_RCVD)))
+#define CONN_HAS(conn, set)	(((conn)->events & (set)) == (set))
 
 static const char *tcp_event_str[] __attribute((__unused__)) = {
 	"SOCK_ACCEPTED", "TAP_SYN_RCVD", "ESTABLISHED", "TAP_SYN_ACK_SENT",
diff --git a/tcp_splice.c b/tcp_splice.c
index b8f64a95..3f9d395a 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -73,9 +73,9 @@ static int ns_sock_pool6	[TCP_SOCK_POOL_SIZE];
 /* Pool of pre-opened pipes */
 static int splice_pipe_pool		[TCP_SPLICE_PIPE_POOL_SIZE][2];
 
-#define CONN_V6(x)			(x->flags & SPLICE_V6)
+#define CONN_V6(x)			((x)->flags & SPLICE_V6)
 #define CONN_V4(x)			(!CONN_V6(x))
-#define CONN_HAS(conn, set)		((conn->events & (set)) == (set))
+#define CONN_HAS(conn, set)		(((conn)->events & (set)) == (set))
 #define CONN(idx)			(&FLOW(idx)->tcp_splice)
 
 /* Display strings for connection events */
-- 
@@ -73,9 +73,9 @@ static int ns_sock_pool6	[TCP_SOCK_POOL_SIZE];
 /* Pool of pre-opened pipes */
 static int splice_pipe_pool		[TCP_SPLICE_PIPE_POOL_SIZE][2];
 
-#define CONN_V6(x)			(x->flags & SPLICE_V6)
+#define CONN_V6(x)			((x)->flags & SPLICE_V6)
 #define CONN_V4(x)			(!CONN_V6(x))
-#define CONN_HAS(conn, set)		((conn->events & (set)) == (set))
+#define CONN_HAS(conn, set)		(((conn)->events & (set)) == (set))
 #define CONN(idx)			(&FLOW(idx)->tcp_splice)
 
 /* Display strings for connection events */
-- 
2.45.2


  parent reply	other threads:[~2024-06-06 10:10 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-06 10:09 [PATCH 0/9] Some more static checker fixes David Gibson
2024-06-06 10:09 ` [PATCH 1/9] tcp: Make pointer const in tcp_revert_seq David Gibson
2024-06-06 10:09 ` [PATCH 2/9] udp: Make rport calculation more local David Gibson
2024-06-06 10:09 ` [PATCH 3/9] cppcheck: Suppress constParameterCallback errors David Gibson
2024-06-07 18:49   ` Stefano Brivio
2024-06-08  6:32     ` David Gibson
2024-06-08 11:48       ` Stefano Brivio
2024-06-06 10:09 ` [PATCH 4/9] Remove pointless macro parameters in CALL_PROTO_HANDLER David Gibson
2024-06-06 10:09 ` David Gibson [this message]
2024-06-06 10:09 ` [PATCH 6/9] util: Use unsigned indices for bits in bitmaps David Gibson
2024-06-06 10:09 ` [PATCH 7/9] conf: Safer parsing of MAC addresses David Gibson
2024-06-06 10:09 ` [PATCH 8/9] lineread: Use ssize_t for line lengths David Gibson
2024-06-06 10:09 ` [PATCH 9/9] util: Use 'long' to represent millisecond durations David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240606100949.1250958-6-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=sbrivio@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).