clang-tidy: Enable the bugprone-macro-parentheses check

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>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

4 files changed, 25 insertions, 26 deletions

diff --git a/Makefile b/Makefile
index 8ea1757..e2180b5 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 2ea0849..26084b4 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 ff1198d..0dccde9 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 b8f64a9..3f9d395 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 */