On 02.05.2024 08:33, Luca Fancellu wrote: > > >> On 2 May 2024, at 07:14, Jan Beulich <[email protected]> wrote: >> >> On 01.05.2024 08:57, Luca Fancellu wrote: >>> Hi Jan, >>> >>>> On 30 Apr 2024, at 12:37, Jan Beulich <[email protected]> wrote: >>>> >>>> On 30.04.2024 13:09, Luca Fancellu wrote: >>>>> --- a/xen/arch/arm/include/asm/setup.h >>>>> +++ b/xen/arch/arm/include/asm/setup.h >>>>> @@ -64,18 +64,20 @@ struct membank { >>>>> }; >>>>> >>>>> struct membanks { >>>>> - unsigned int nr_banks; >>>>> - unsigned int max_banks; >>>>> + __struct_group(membanks_hdr, common, , >>>>> + unsigned int nr_banks; >>>>> + unsigned int max_banks; >>>>> + ); >>>>> struct membank bank[]; >>>>> }; >>>> >>>> I'm afraid I can't spot why __struct_group() is needed here. Why would just >>>> one of the two more straightforward >>>> >>>> struct membanks { >>>> struct membanks_hdr { >>>> unsigned int nr_banks; >>>> unsigned int max_banks; >>>> ); >>>> struct membank bank[]; >>>> }; >>>> >>> >>> At the first sight I thought this solution could have worked, however GCC >>> brought me back down to earth >>> remembering me that flexible array members can’t be left alone in an empty >>> structure: >>> >>> /data_sdc/lucfan01/gitlab_mickledore_xen/xen/xen/arch/arm/include/asm/setup.h:70:6: >>> error: declaration does not declare anything [-Werror] >>> 70 | }; >>> | ^ >>> /data_sdc/lucfan01/gitlab_mickledore_xen/xen/xen/arch/arm/include/asm/setup.h:71:20: >>> error: flexible array member in a struct with no named members >>> 71 | struct membank bank[]; >>> | ^~~~ >>> [...] >> >> Since for patch 1 you looked at Linux'es uapi/linux/stddef.h, the solution >> to this lies there, in __DECLARE_FLEX_ARRAY(). Alongside or instead of >> borrowing __struct_group(), we could consider borrowing this as well. Or >> open-code it just here, for the time being (perhaps my preference). Yet >> it's not clear to me that doing so will actually be enough to make things >> work for you. > > I looked also into __DECLARE_FLEX_ARRAY(), but then decided __struct_group() > was enough for my purpose, can I ask the technical reasons why it would be > your > preference? Is there something in that construct that is a concern for you?
I don't like either construct very much, but of the two __DECLARE_FLEX_ARRAY() looks slightly more "natural" for what is wanted and how it's done. __struct_group() introducing twice the (effectively) same structure feels pretty odd, for now at least. It's not even entirely clear to me whether there aren't pitfalls, seeing that the C spec differentiates named and unnamed struct fields in a few cases. For __DECLARE_FLEX_ARRAY(), otoh, I can't presently see any reason to suspect possible corner cases. Yet as said before - I'm not sure __DECLARE_FLEX_ARRAY() alone would be enough for what you want to achieve. Jan
