Re: [PATCH] target: pscsi: avoid Wempty-body warning

[Arnd Bergmann]

On Mon, Mar 22, 2021 at 4:53 PM Willy Tarreau <w@xxxxxx> 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@xxxxxxxx>
> > >
> > > 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