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.