lkml.org 
[lkml]   [2022]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2] netfilter: conntrack: Fix clang -Wformat warning in print_tuple()
    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

    $ ./scripts/get_maintainer.pl
    0001-netfilter-conntrack-Fix-clang-Wformat-warning-in-pri.patch
    Pablo Neira Ayuso <pablo@netfilter.org> (maintainer:NETFILTER)
    Jozsef Kadlecsik <kadlec@netfilter.org> (maintainer:NETFILTER)
    Florian Westphal <fw@strlen.de> (maintainer:NETFILTER)
    "David S. Miller" <davem@davemloft.net> (maintainer:NETWORKING [GENERAL])
    Eric Dumazet <edumazet@google.com> (maintainer:NETWORKING [GENERAL])
    Jakub Kicinski <kuba@kernel.org> (maintainer:NETWORKING [GENERAL])
    Paolo Abeni <pabeni@redhat.com> (maintainer:NETWORKING [GENERAL])
    Nathan Chancellor <nathan@kernel.org> (supporter:CLANG/LLVM BUILD SUPPORT)
    Nick Desaulniers <ndesaulniers@google.com> (supporter:CLANG/LLVM BUILD SUPPORT)
    Tom Rix <trix@redhat.com> (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 <ndesaulniers@google.com> wrote:
    >
    > On Tue, Jun 7, 2022 at 11:09 AM Justin Stitt <jstitt007@gmail.com> wrote:
    > >
    > > | 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.
    >
    > >
    > > Signed-off-by: Justin Stitt <jstitt007@gmail.com>
    > > ---
    > > 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

    \
     
     \ /
      Last update: 2022-06-08 04:55    [W:3.424 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site