https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81484

--- Comment #5 from Arnd Bergmann <arnd at linaro dot org> ---
(In reply to Marek Polacek from comment #4)
> (In reply to Arnd Bergmann from comment #3)
> > It seems I got a little confused when I only looked at the initial patch
> > that was proposed, which was supposed to cover specifically the comparison
> > followed by ?: as in
> > https://gcc.gnu.org/ml/gcc-patches/2016-09/msg00115.html. I see now that the
> > patch that made it into the release also covers the more general case and
> > that seems intentional, it just causes false-positive warnings.
> 
> I agree it's just a style warning here (to warn about if (c ? 0 : 2) I mean,
> as that's correct code), but the warning allows you to simplify the code, so
> that seems to be a good thing.

Right, in the particular case I ran into, the only thing that prevents
me from rewriting

 setsign(dest, ((control_w & CW_RC) != RC_DOWN) ? SIGN_POS : SIGN_NEG);

into

 setsign(dest, (control_w & CW_RC) == RC_DOWN);

is the local policy of always passing either SIGN_POS or SIGN_NEG
into setsign(), rather than a boolean. Similar code might rely
on those calling conventions, but this file is the only such instance
I found in Linux, so that is not a reason to change the compiler.

> > The part that still seems odd here is that the warning does not take into
> > account the macro: The caller just passes an expression as the integer
> > argument into the setsign() macro, while the macro tests it for being
> > nonzero.
> > 
> > In the opposite case, I found a piece of kernel code that does not produce a
> > warning unless we preprocess it first (as ccache does for instance):
> > 
> > #define EINVAL 22
> > #define v4l2_subdev_call(sd, f) ((sd)->f ? (sd)->f(sd) : -EINVAL)
> > struct v4l2_subdev {
> >         int (*g_sliced_fmt)(struct v4l2_subdev *sd);
> > };
> > int f(struct v4l2_subdev *sd)
> > {
> >         if (v4l2_subdev_call(sd, g_sliced_fmt))
> >                 return -EINVAL;
> > 
> >         return 0;
> > }
> > 
> > Here the macro contains the ?: operator while the caller contains the
> > 'if()', and something in gcc seems to decide not to warn about this, but
> > that logic does not identify the first example as an false-positive.
> 
> The warning checks whether the ?: expression comes from a macro.  If it does
> (as in Comment 3), it doesn't warn.  But in the original testcase the ?:
> itself is not coming from a macro so it warns.

Ok, thanks for the explanation and for your time.

Reply via email to