Re: [PATCH v2] netfilter: conntrack: Fix clang -Wformat warning in print_tuple()
From: Nick Desaulniers <ndesaulniers@google.com>
Date: 2022-06-08 02:03:34
Also in:
lkml, llvm, netfilter-devel
Also, please make sure to run scripts/get_maintainer.pl on your patch file and CC everyone it recommends, with the maintainers you want to pick up your patch in the To field. https://lore.kernel.org/llvm/20220607180847.13482-1-jstitt007@gmail.com/T/#u (local) $ ./scripts/get_maintainer.pl 0001-netfilter-conntrack-Fix-clang-Wformat-warning-in-pri.patch Pablo Neira Ayuso [off-list ref] (maintainer:NETFILTER) Jozsef Kadlecsik [off-list ref] (maintainer:NETFILTER) Florian Westphal [off-list ref] (maintainer:NETFILTER) "David S. Miller" [off-list ref] (maintainer:NETWORKING [GENERAL]) Eric Dumazet [off-list ref] (maintainer:NETWORKING [GENERAL]) Jakub Kicinski [off-list ref] (maintainer:NETWORKING [GENERAL]) Paolo Abeni [off-list ref] (maintainer:NETWORKING [GENERAL]) Nathan Chancellor [off-list ref] (supporter:CLANG/LLVM BUILD SUPPORT) Nick Desaulniers [off-list ref] (supporter:CLANG/LLVM BUILD SUPPORT) Tom Rix [off-list ref] (reviewer:CLANG/LLVM BUILD SUPPORT) netfilter-devel@vger.kernel.org (open list:NETFILTER) coreteam@netfilter.org (open list:NETFILTER) netdev@vger.kernel.org (open list:NETWORKING [GENERAL]) linux-kernel@vger.kernel.org (open list) llvm@lists.linux.dev (open list:CLANG/LLVM BUILD SUPPORT On Tue, Jun 7, 2022 at 1:33 PM Nick Desaulniers [off-list ref] wrote:
On Tue, Jun 7, 2022 at 11:09 AM Justin Stitt [off-list ref] wrote:quoted
| net/netfilter/nf_conntrack_standalone.c:63:7: warning: format specifies type | 'unsigned short' but the argument has type 'int' [-Wformat] | ntohs(tuple->src.u.tcp.port), | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ | net/netfilter/nf_conntrack_standalone.c:64:7: warning: format specifies type | 'unsigned short' but the argument has type 'int' [-Wformat] | ntohs(tuple->dst.u.tcp.port)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ | net/netfilter/nf_conntrack_standalone.c:69:7: warning: format specifies type | 'unsigned short' but the argument has type 'int' [-Wformat] | ntohs(tuple->src.u.udp.port), | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ | net/netfilter/nf_conntrack_standalone.c:70:7: warning: format specifies type | 'unsigned short' but the argument has type 'int' [-Wformat] | ntohs(tuple->dst.u.udp.port)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ | net/netfilter/nf_conntrack_standalone.c:75:7: warning: format specifies type | 'unsigned short' but the argument has type 'int' [-Wformat] | ntohs(tuple->src.u.dccp.port), | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | net/netfilter/nf_conntrack_standalone.c:76:7: warning: format specifies type | 'unsigned short' but the argument has type 'int' [-Wformat] | ntohs(tuple->dst.u.dccp.port)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | net/netfilter/nf_conntrack_standalone.c:80:7: warning: format specifies type | 'unsigned short' but the argument has type 'int' [-Wformat] | ntohs(tuple->src.u.sctp.port), | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | net/netfilter/nf_conntrack_standalone.c:81:7: warning: format specifies type | 'unsigned short' but the argument has type 'int' [-Wformat] | ntohs(tuple->dst.u.sctp.port)); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Variadic functions (printf-like) undergo default argument promotion. Documentation/core-api/printk-formats.rst specifically recommends using the promoted-to-type's format flag. Also, as per C11 6.3.1.1: (https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1548.pdf) `If an int can represent all values of the original type ..., the value is converted to an int; otherwise, it is converted to an unsigned int. These are called the integer promotions.` Thus it makes sense to change %hu (as well as %u) to %d. It should be noted that %u does not produce the same warning as %hu in this context. However, it should probably be changed as well for consistency.Right, because they are `unsigned char` and the parameter is unnamed for variadic functions they are also default-argument-promoted to int. -Wformat won't warn on signedness. Thanks for the patch! Reviewed-by: Nick Desaulniers <ndesaulniers@google.com> Link: https://github.com/ClangBuiltLinux/linux/issues/378 Also, Nathan supplied his RB tag on v1; it's ok next time to include it on subsequent revisions of patches, so long as you don't change the patch too much between revisions.quoted
Signed-off-by: Justin Stitt <redacted> --- Diff between v1 -> v2: * update commit message and subject line Note: The architecture (arm64) is critical for reproducing this warning. net/netfilter/nf_conntrack_standalone.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)diff --git a/net/netfilter/nf_conntrack_standalone.c b/net/netfilter/nf_conntrack_standalone.c index 6ad7bbc90d38..afbec8a12c5e 100644 --- a/net/netfilter/nf_conntrack_standalone.c +++ b/net/netfilter/nf_conntrack_standalone.c@@ -53,30 +53,30 @@ print_tuple(struct seq_file *s, const struct nf_conntrack_tuple *tuple, switch (l4proto->l4proto) { case IPPROTO_ICMP: - seq_printf(s, "type=%u code=%u id=%u ", + seq_printf(s, "type=%d code=%d id=%d ", tuple->dst.u.icmp.type, tuple->dst.u.icmp.code, ntohs(tuple->src.u.icmp.id)); break; case IPPROTO_TCP: - seq_printf(s, "sport=%hu dport=%hu ", + seq_printf(s, "sport=%d dport=%d ", ntohs(tuple->src.u.tcp.port), ntohs(tuple->dst.u.tcp.port)); break; case IPPROTO_UDPLITE: case IPPROTO_UDP: - seq_printf(s, "sport=%hu dport=%hu ", + seq_printf(s, "sport=%d dport=%d ", ntohs(tuple->src.u.udp.port), ntohs(tuple->dst.u.udp.port)); break; case IPPROTO_DCCP: - seq_printf(s, "sport=%hu dport=%hu ", + seq_printf(s, "sport=%d dport=%d ", ntohs(tuple->src.u.dccp.port), ntohs(tuple->dst.u.dccp.port)); break; case IPPROTO_SCTP: - seq_printf(s, "sport=%hu dport=%hu ", + seq_printf(s, "sport=%d dport=%d ", ntohs(tuple->src.u.sctp.port), ntohs(tuple->dst.u.sctp.port)); break; --2.30.2-- Thanks, ~Nick Desaulniers
-- Thanks, ~Nick Desaulniers