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

[Nick Desaulniers <ndesaulniers@google.com>]

On Wed, Apr 26, 2023 at 11:11 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Wed, Apr 26, 2023 at 10:34 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > That's what clang's _Nonnull attribute does (with -Wnullability-extension).
>
> No, that's a warning about using it, not a warning about testing for
> NULL when it's there.
>
> I actually tested _Nonnull.  It seems to work for arguments. But it
> does not work for return values.

Ah, it does do something in the callee, not the caller:
https://godbolt.org/z/9dsPKGMWq

But I see your point; it would be nice to flag that the comparison
against NULL seems suspicious.

>
> Of course, maybe there's some other magic needed, but it does seem to
> be sadly not working for us.
>
> > But it's not toolchain portable, at the moment.  Would require changes
> > to clang to use the GNU C __attribute__ syntax, too (which I'm not
> > against adding support for).
>
> No need for using the __attribute__ syntax at all, that would _not_ be
> a show-stopper.

Ack.

>
> While it's true that it's the common syntax, and we sometimes use it
> explicitly because of that, it's by no means the only syntax, and we
> actually tend to try to have more legible wrappers around it.
>
> So, for example, we prefer using '__weak' instead of writing
> '__attribute__((__weak__))'.
>
> And no, it very much doesn't have to use __attibute__ at all. We
> already have things like
>
>     #define __diag(s)          _Pragma(__diag_str(GCC diagnostic s))
>
> so we already use other syntaxes.
>
> End result: if it actually worked, I'd happily do something like
>
>    #define __return_nonnull _Nonnull
>
> in <linux/compiler-clang.h>, with then <linux/compiler-gcc.h> then just having
>
>      #define __return_nonnull
>
> along with a big comment about how __attribute__((nonnull)) is
> horrible garbage that should never every be used.
>
>              Linus



-- 
Thanks,
~Nick Desaulniers