Re: [Intel-wired-lan] [RFC net-next 1/2] overflow: add DECLARE_FLEX() for on-stack allocs

[Alexander Lobakin <aleksander.lobakin@intel.com>]

From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Date: Tue, 1 Aug 2023 13:19:22 +0200

> Add DECLARE_FLEX() macro for on-stack allocations of structs with
> flexible array member.
> 
> Using underlying array for on-stack storage lets us to declare known
> on compile-time structures without kzalloc().
> 
> Actual usage for ice driver is in next patch of the series.
> 
> Note that "struct" kw and "*" char is moved to the caller, to both:
> have shorter macro name, and have more natural type specification
> in the driver code (IOW not hiding an actual type of var).
> 
> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> ---
>  include/linux/overflow.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index f9b60313eaea..403b7ec120a2 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -309,4 +309,18 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
>  #define struct_size_t(type, member, count)					\
>  	struct_size((type *)NULL, member, count)
>  
> +/**
> + * DECLARE_FLEX() - Declare an on-stack instance of structure with trailing
> + * flexible array.
> + * @type: Pointer to structure type, including "struct" keyword and "*" char.
> + * @name: Name for a (pointer) variable to create.
> + * @member: Name of the array member.
> + * @count: Number of elements in the array; must be compile-time const.
> + *
> + * Declare an instance of structure *@type with trailing flexible array.
> + */
> +#define DECLARE_FLEX(type, name, member, count)					\
> +	u8 name##_buf[struct_size((type)NULL, member, count)] __aligned(8) = {};\

1. You can use struct_size_t() instead of open-coding it.
2. Maybe use alignof(type) instead of 8? Some structures have larger
   alignment requirements.

> +	type name = (type)&name##_buf

In general, I still think DECLARE_FLEX(struct foo) is better than
DECLARE_FLEX(struct foo *). Looking at container_of(), struct_size_t()
etc., they all take `type`, not `type *`, so even from the consistency
perspective your solution is not optimal to me.

> +
>  #endif /* __LINUX_OVERFLOW_H */

Thanks,
Olek
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan