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

--- Comment #64 from Alejandro Colomar <alx at kernel dot org> ---
(In reply to Kees Cook from comment #63)
> (In reply to Alejandro Colomar from comment #62)
> > What's the value of returning NULL instead of just failing the compilation
> > with an error?
> 
> It's so that the same allocator macros can be used for FAM structs with or
> without a "counted_by" attribute. If "counted_by" is marked, the counter is
> updated. If it doesn't exist, it is not.
> 
> Here's the expected usage from comment #1 (adjusted for the new API):
> 
>   #define alloc(P, FAM, COUNT, GFP) ({ \
>     typeof(P) __p; \
>     size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
>     __p = kmalloc(__size, GFP); \
>     if (__p && __builtin_get_counted_by(__p->FAM)) \
>       *__builtin_get_counted_by(__p->FAM) = COUNT; \
>     __p; \
>   })

How about having two macros?  One that works for non-attributed pointers, and
other that works for attributed ones.  And use the appropriate one for each of
them.

If you accidentally use the one that uses the builtin, it'll cause a compiler
error.

#define alloc(P, FAM, COUNT, GFP) \
({ \
    typeof(P) __p; \
    __p = alloc_not_counted_by(P, FAM, COUNT, GFP); \
    *__builtin_get_counted_by(__p->FAM) = COUNT; \
    __p; \
})

alloc_no_attribute() would work everywhere, and alloc() only where the
attribute is used.

> 
> We can actually take this even further and sanity check "COUNT" against the
> counter type in addition to setting it:
> 
>   #define alloc(P, FAM, COUNT, GFP) ({ \
>     size_t __count = (COUNT); \
>     typeof(P) __p; \
>     if (__builtin_get_counted_by(__p->FAM) && \
>         __count > type_max(typeof(*__builtin_get_counted_by(__p->FAM))) { \
>       __p = NULL; \
>     } else { \
>       size_t __size = struct_size(*__p, FAM, __count); \
>       __p = kmalloc(__size, GFP); \
>       if (__p && __builtin_get_counted_by(__p->FAM)) \
>         *__builtin_get_counted_by(__p->FAM) = __count; \
>     }
>     __p; \
>   })

You could still do this with my approach.  And there are less NULL pointers
around, which would get messy.

Reply via email to