lkml.org 
[lkml]   [2022]   [Aug]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [for-linus][PATCH 01/10] tracing: Suppress sparse warnings triggered by is_signed_type()
    From
    On 8/21/22 11:35, Linus Torvalds wrote:
    > If we want to just shut up the sparse warning, then afaik the simple
    > one-liner fix would have been
    >
    > -#define is_signed_type(type) (((type)(-1)) < (type)1)
    > +#define is_signed_type(type) (((__force type)(-1)) < (__force type)1)
    >
    > and at least then sparse checks the same source as is compiled,
    > instead of passing a "this is not a signed type" to places.

    Hi Linus,

    I agree that it's better that sparse sees the same code as what is used to
    build the kernel. However, I do not agree that the patch above is a solution.
    Sparse reports a warning for the suggested definition above of is_signed_type()
    because the new definition tries to use the less-than (<) operator to compare
    two __bitwise types.

    > Btw, that patch is entirely broken for *another* reason.
    >
    > Even if you were to say "ok, sparse just gets a different argument",
    > the fact that the trace_events file re-defined that is_signed_type()
    > macro means that you added that
    >
    > +#undef is_signed_type
    >
    > to make the compiler happy about how you only modified one of them.
    >
    > But that then means that if <linux/trace_events.h> gets included
    > *before* <linux/overflow.h>, you'll just get the warning *there*
    > instead.
    >
    > Now, that warning would only happen for a __CHECKER__ build - but
    > that's the only build this patch is relevant for anyway.
    >
    > And maybe that ordering doesn't exist, or maybe it only exists on some
    > very random config. Regardless, it's broken.
    >
    > Of course, the real fix should be to just not re-define that macro at
    > all, and just have it in *one* place.

    Agreed that is_signed_type() should only be defined once. If nobody else
    beats me to this I will prepare a patch to fix this.

    Thanks,

    Bart.

    \
     
     \ /
      Last update: 2022-08-22 20:20    [W:4.443 / U:0.108 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site