Re: [GIT PULL] ext4 changes for the 6.4 merge window

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Apr 26, 2023 at 3:08 PM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> Is this what you had in mind?
> ```
> void * _Nonnull foo (void)
> ...
> void bar (void) {
>     if (foo() == NULL) // maybe should warn that foo() returns _Nonnull?
>         bar();
> ...
> linus.c:8:15: warning: comparison of _Nonnull function call 'foo'
> equal to a null pointer is always false

Yes.

HOWEVER.

I suspect you will find that it gets complicated for more indirect
uses, and that may be why people have punted on this.

For example, let's say that you instead have

   void *bar(void) { return foo(); }

and 'bar()' gets inlined.

The obvious reaction to that is "ok, clearly the result is still
_Nonnull, and should warn if it is tested.

But that obvious reaction is actually completely wrong, because it may
be that the real code looks something like

   void *bar(void) {
#if CONFIG_XYZ
    if (somecondition) return NULL;
#endif
    return foo(); }

and the caller really *should* check for NULL - it's just that the
compiler never saw that case.

So only testing the direct return value of a function should warn.

And even that "direct return value" is not trivial. What happens if
you have something like this:

   void bar(void) { do_something(foo()); }

and "do_something()" ends up being inlined - and checks for its
argument for being NULL? Again, that "test against NULL" may well be
absolutely required in that context - because *other* call-sites will
pass in pointers that might be NULL.

Now, I don't know how clang works internally, but I suspect based just
on the size of your patch that your patch would get all of this
horribly wrong.

So doing a naked

    void *ptr = foo();
    if (!ptr) ...

should warn.

But doing the exact same thing, except the test for NULL came in some
other context that just got inlined, cannot warn.

I _suspect_ that the reason clang does what it does is that this is
just very complicated to do well.

It sounds easy to a human, but ...

          Linus




[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux