Re: [patch 9/9] mm/vmalloc: add __alloc_size attributes for better bounds checking

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

 



On Thu, Sep 9, 2021 at 8:10 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> +__alloc_size(1)
>  extern void *vmalloc(unsigned long size);
[...]

All of these are added in the wrong place - inconsistent with the very
compiler documentation the patches add.

The function attributes are generally added _after_ the function,
although admittedly we've been quite confused here before.

But the very compiler documentation you point to in the patch that
adds these macros gives that as the examples both for gcc and clang:

+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alloc_005fsize-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#alloc-size

and honestly I think that is the preferred format because this is
about the *function*, not about the return type.

Do both placements work? Yes.

Have we been confused about this before? Yes. I note that our __printf
attributes in particular have been added in odd places. And our
existing __malloc annotations seem to correct in <linux/slab.h> and
<linux/device.h> but then randomly applied in some other places.

I really think it's pointlessly stupid and hard to read/grep for to
make it be a separate line before the whole thing.

I also think it should have taken over the "__malloc" name that is
almost unused right now. Because why would you ever have
__alloc_size() without having __malloc().

So wouldn't this all have looked much more natural as

     void *vmalloc(unsigned long size) __malloc(1);

instead? Hmm?

              Linus



[Index of Archives]     [Kernel Archive]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux