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

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 8/1/23 15:54, Alexander Lobakin wrote:
> 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.

with ptr param, not feasible, but otherwise, of course will do it (see 
below)

> 2. Maybe use alignof(type) instead of 8? Some structures have larger
>     alignment requirements.

Sure, thanks!

> 
>> +	type name = (type)&name##_buf
> 
> In general, I still think DECLARE_FLEX(struct foo) is better than
> DECLARE_FLEX(struct foo *).

I have started with that version, and that would prevent your question 
no. 1 :) So there is additional advantage to that.

> 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.

The two you have mentioned are "getter" macros. Random two from me, that 
actually declare something are:

#define DEVICE_ATTR_RW(_name) \
	struct device_attribute dev_attr_##_name = __ATTR_RW(_name)

#define DECLARE_BITMAP(name, bits) \
	unsigned long name[BITS_TO_LONGS(bits)]

Even if they don't take @type param, they declare variable of some 
non-pointer type.

Both variants have some logic that supports them, and some disadvantages:
ptr-arg: user declares sth as ptr, but it takes "a lot" of space
just-type-arg: user declares foo, but it's "*foo" actually, so "foo.bar" 
does not work.

I have no strong opinion here, so will just switch to pure-type param.

> Thanks,
> Olek

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