> On 2 May 2024, at 07:09, Jan Beulich <jbeul...@suse.com> wrote:
>
> On 01.05.2024 08:54, Luca Fancellu wrote:
>>> On 30 Apr 2024, at 12:43, Jan Beulich <jbeul...@suse.com> wrote:
>>> On 30.04.2024 13:09, Luca Fancellu wrote:
>>>> --- a/xen/include/xen/kernel.h
>>>> +++ b/xen/include/xen/kernel.h
>>>> @@ -54,6 +54,27 @@
>>>> typeof_field(type, member) *__mptr = (ptr); \
>>>> (type *)( (char *)__mptr - offsetof(type,member) );})
>>>>
>>>> +/**
>>>> + * __struct_group() - Create a mirrored named and anonyomous struct
>>>> + *
>>>> + * @TAG: The tag name for the named sub-struct (usually empty)
>>>> + * @NAME: The identifier name of the mirrored sub-struct
>>>> + * @ATTRS: Any struct attributes (usually empty)
>>>> + * @MEMBERS: The member declarations for the mirrored structs
>>>> + *
>>>> + * Used to create an anonymous union of two structs with identical layout
>>>> + * and size: one anonymous and one named. The former's members can be used
>>>> + * normally without sub-struct naming, and the latter can be used to
>>>> + * reason about the start, end, and size of the group of struct members.
>>>> + * The named struct can also be explicitly tagged for layer reuse, as well
>>>> + * as both having struct attributes appended.
>>>> + */
>>>> +#define __struct_group(TAG, NAME, ATTRS, MEMBERS...) \
>>>> + union { \
>>>> + struct { MEMBERS } ATTRS; \
>>>> + struct TAG { MEMBERS } ATTRS NAME; \
>>>> + } ATTRS
>>>
>>> Besides my hesitance towards having this construct, can you explain why
>>> ATTR needs using 3 times, i.e. also on the wrapping union?
>>
>> The original commit didn’t have the third ATTRS, but afterwards it was
>> introduced due to
>> this:
>>
>> https://patchwork.kernel.org/project/linux-wireless/patch/20231120110607.98956-1-dmanti...@yandex.ru/#25610045
>
> Hmm. Since generally packing propagates to containing compound types, I'd
> say this cannot go without a code comment, or at the very least a mention
> in the description. But: Do we use this old ABI in Xen at all? IOW can we
> get away without this 3rd instance?
Yes, I think it won’t be a problem for Xen, is it something that can be done on
commit?
Anyway in case of comments on the second patch, I’ll push the change also for
this one.
Cheers,
Luca
