linux-kernel - Re: [PATCH] target: pscsi: 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] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 22 Mar 2021 17:18:23 +0100
From:   Arnd Bergmann <arnd@...nel.org>
To:     Willy Tarreau <w@....eu>
Cc:     Christoph Hellwig <hch@...radead.org>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Jens Axboe <axboe@...nel.dk>,
        Chaitanya Kulkarni <chaitanya.kulkarni@....com>,
        Hannes Reinecke <hare@...e.de>,
        Bodo Stroesser <bstroesser@...fujitsu.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        Guoqing Jiang <guoqing.jiang@...ud.ionos.com>,
        linux-scsi <linux-scsi@...r.kernel.org>,
        target-devel@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] target: pscsi: avoid Wempty-body warning

On Mon, Mar 22, 2021 at 4:53 PM Willy Tarreau <w@....eu> wrote:
> On Mon, Mar 22, 2021 at 03:47:35PM +0000, Christoph Hellwig wrote:
> > On Mon, Mar 22, 2021 at 12:44:34PM +0100, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@...db.de>
> > >
> > > Building with 'make W=1' shows a harmless warning for pscsi:
> > >
> > > drivers/target/target_core_pscsi.c: In function 'pscsi_complete_cmd':
> > > drivers/target/target_core_pscsi.c:624:33: error: suggest braces around empty body in an 'if' statement [-Werror=empty-body]
> > >   624 |                                 ; /* XXX: TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE */
> > >       |                                 ^
> > >
> > > Rework the coding style as suggested by gcc to avoid the warning.
> >
> > I would much, much prefer to drop the bogus warning;
> >
> >       if (foo)
> >               ; /* comment */
> >
> > is a fairly usual and absolutely sensible style.  The warning on hte
> > other hand is completely stupid.
>
> Agreed!
>
> These days it seems there is a competition for the stupidest warning
> between compilers, and we've reached the point where working around
> them manages to introduce real bugs :-(
>
> I predict we'll soon see warning such as "this comment looks like valid
> C code, if you really intended to comment it out, use #if 0 instead". Oh
> well, let's hope I have not given a new idea here...

I agree that this instance of the warning is particularly stupid, but the
I'd like to leave the warning option there and eventually enable it by
default because it tends to find other more interesting cases, and this
one is trivial to work around.

I remember previously fixing a few drivers that did obviously
incorrect things like

    if (error); /* note the extra ';' */
         return error;

and a lot mostly harmless code like

#ifdef DEBUG_THIS_DRIVER /* always disabled */
#define dprintk(args...) printk(args)
#else
#define dprintk(args...)
#endif
    /* note the mismatched format string */
    dprintk(KERN_WARNING "error %d\n", &object);

Turning the empty dprintk() into no_printk() means we can catch
the wrong format string during compile testing.

        Arnd