From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 072825A0059 for ; Thu, 06 Jun 2024 12:10:09 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1717668601; bh=DgPVP8b+3nnLvjEny4yZ4bDcRJhEdrqm+SKMrjNIIVY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Xam1FreowW7c4BbmJ0iaD48H0trqJmb0G82rUYiJ80DVXuaMy2l5OdI8kqqcRfhxo jFsyK3sU4oKTKFLRUkkR6ComDx+k5SDyXevBo6gKR+K17UdVH1b/isvgVCBPQdaLuv fHM6sPl0gsWcG0Cof8hhoVYdnR7SocrYXBD/mARyy/h0/wr7HSPqTrGt2TnCJftvWm wL5chZN1SnHWXMXhXoOD30fE3GkIwLfY+B0Lfkflc3W/h9O4J9v0KbwK/+HkzP7JAe gVTrxZC3k7Opu5M2MOHZ8bM9v68wCPfVg/B1dr/dRRxs+5L0AUwW40x2tFAHSLiaG0 H3vB5jOzkqF5A== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4Vw0ST566cz4x2P; Thu, 6 Jun 2024 20:10:01 +1000 (AEST) From: David Gibson To: passt-dev@passt.top, Stefano Brivio Subject: [PATCH 5/9] clang-tidy: Enable the bugprone-macro-parentheses check Date: Thu, 6 Jun 2024 20:09:45 +1000 Message-ID: <20240606100949.1250958-6-david@gibson.dropbear.id.au> X-Mailer: git-send-email 2.45.2 In-Reply-To: <20240606100949.1250958-1-david@gibson.dropbear.id.au> References: <20240606100949.1250958-1-david@gibson.dropbear.id.au> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Message-ID-Hash: VSYERIWPR6CW375ESRD7PUKE5VANFL2G X-Message-ID-Hash: VSYERIWPR6CW375ESRD7PUKE5VANFL2G X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: David Gibson X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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 */ -- 2.45.2