linux-kernel - Re: [PATCH] posix-acl: avoid -Wempty-body warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 22 Mar 2021 14:08:13 +0100
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        James Morris <jamorris@...ux.microsoft.com>,
        Serge Hallyn <serge@...lyn.com>,
        Linux FS-devel Mailing List <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] posix-acl: avoid -Wempty-body warning

On Mon, Mar 22, 2021 at 02:02:54PM +0100, Arnd Bergmann wrote:
> On Mon, Mar 22, 2021 at 1:15 PM Christian Brauner
> <christian.brauner@...ntu.com> wrote:
> > On Mon, Mar 22, 2021 at 12:38:24PM +0100, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@...db.de>
> > >
> > > The fallthrough comment for an ignored cmpxchg() return value
> > > produces a harmless warning with 'make W=1':
> > >
> > > fs/posix_acl.c: In function 'get_acl':
> > > fs/posix_acl.c:127:36: error: suggest braces around empty body in an 'if' statement [-Werror=empty-body]
> > >   127 |                 /* fall through */ ;
> > >       |                                    ^
> > >
> > > Rewrite it as gcc suggests as a step towards a clean W=1 build.
> > > On most architectures, we could just drop the if() entirely, but
> > > in some cases this causes a different warning.
> >
> > And you don't see the warning for the second unconditional
> > cmpxchg(p, sentinel, ACL_NOT_CACHED);
> > below?
> 
> I would have expected both to show that warning, didn't notice the other
> one.  I now see that all architectures use statement expressions for cmpxchg()
> and xchg() these days, after we fixed m68k, alpha and ia64, so the
> changelog text here no longer makes sense.
> 
> Should I just remove the if() then?

I think so. It seems like the straightforward thing to do. The comment
above this cmpxchg() also explains clearly what the expectations are.
At least to me the visual hint due to the "!= ACL_NOT_CACHED" check in
the if condition doesn't provide any additional clarity.

Christian