Re: [PATCH 3/3] docs/doxygen: doxygen documentation for grant_table.h

[Luca Fancellu <luca.fancellu@xxxxxxx>]

> On 7 Apr 2021, at 22:26, Stefano Stabellini <sstabellini@xxxxxxxxxx> 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;
};

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

Moreover the change should be back-compatible since we are not changing member 
names.

Cheers,

Luca