> On 8 Apr 2021, at 12:40, Julien Grall <jul...@xen.org> wrote:
>
> Hi Luca,
>
> On 08/04/2021 12:02, Luca Fancellu wrote:
>>> On 7 Apr 2021, at 22:26, Stefano Stabellini <sstabell...@kernel.org> wrote:
>>>
>>> On Wed, 7 Apr 2021, Jan Beulich wrote:
>>>> On 07.04.2021 10:42, Luca Fancellu wrote:
>>>>> Just to be sure that we are in the same page, are you suggesting to
>>>>> modify the name
>>>>> In this way?
>>>>>
>>>>> struct gnttab_cache_flush {
>>>>> - union {
>>>>> + union xen_gnttab_cache_flush_a {
>>>>> uint64_t dev_bus_addr;
>>>>> grant_ref_t ref;
>>>>> } a;
>>>>>
>>>>> Following this kind of pattern: xen_<upper struct name>_<member name> ?
>>>>
>>>> While in general I would be fine with this scheme, for field names like
>>>> "a" or "u" it doesn't fit well imo.
>>>
>>> "a" is a bad name anyway, even for the member. We can take the
>>> opportunity to find a better name. Almost anything would be better than
>>> "a". Maybe "refaddr"?
>>>
>>>
>>>> I'm also unconvinced this would be
>>>> scalable to the case where there's further struct/union nesting.
>>>
>>> How many of these instances of multilevel nesting do we have? Luca might
>>> know. Probably not many? They could be special-cased.
>> There are not many multilevel nesting instances of anonymous struct/union
>> and the maximum level of nesting I found in the public headers is 2:
>> union {
>> union/struct {
>> …
>> } <name>
>> } <name>
>> I also see that in the majority of cases the unions have not meaningful
>> names like “a” or “u” as member name, instead struct names are fine,
>> It could be fine to keep the meaningful name the same for the struct type
>> name and use the pattern for the non-meaningful ones as long
>> as the names doesn’t create compilation errors?
>> Example:
>> struct upper_level {
>> union {
>> struct {
>>
>> } meaningful_name1;
>> struct {
>>
>> } meaningful_name2;
>> } u;
>> };
>> becomes:
>> struct upper_level {
>> union upper_level_u {
>> struct meaningful_name1 {
>>
>> } meaningful_name1;
>> struct meaningful_name2 {
>>
>> } meaningful_name2;
>> } u;
>> };
>
> If I understand correctly your proposal, the name of the structure would be
> the name of the field. The name of the fields are usually pretty generic so
> you will likely end up to redefine the structure name.
>
> Unless we want to provide random name, the only safe naming would be to
> define the structure as upper_level_u_meaningful_name{1, 2}. But, this is
> going to be pretty awful to read.
>
> But I am still a bit puzzled by the fact doxygen is not capable to deal with
> anynomous/unamed union. How do other projects deal with them?
>
>> Doing this will help a lot the documentation side because the html page will
>> show the "struct upper_level" with inside the “union upper_level_u" and
>> inside again
>> the two struct “meaningful_name1" and “meaningful_name2", and from the point
>> of view of the developer it can tell her/him exactly the name of the member
>> to
>> access when writing code (apart from the upper_level_u that can be accessed
>> by u, but we can’t clearly change it).
>
> I don't quite understand the last point. Wouldn't the developper see the
> field name? So how is it going to be different from seeing the structure name?
The developer, that is using the documentation generated with sphinx+doxygen,
will see the structure name and not the field name because this is the way
sphinx+doxygen is rendering the code structures. You can see it in the
generated documentation using this serie.
>
>> If this sounds difficult to understand when reading, please generate the
>> documentation and have a look on the page in one side and the source code in
>> another.
>
> Just to be clear, do you mean understanding what you wrote or a developper
> trying to understand the code?
I meant understanding what I wrote, because I know it’s difficult to describe
the concept without visualising the html page, it would have been much easier
to attach
an image to clarify.
>
> Cheers,
>
> --
> Julien Grall
