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

[Julien Grall <julien@xen.org>]

On 08/04/2021 12:40, Julien Grall wrote:
> Hi Luca,
> 
> On 08/04/2021 12:02, Luca Fancellu wrote:
>>
>>
>>> On 7 Apr 2021, at 22:26, Stefano Stabellini <sstabellini@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?

While going through the list of anynomous union in Xen, I noticed we 
also have something like:

struct test {
     union {
         int a;
         int b;
     };
};

We can't name them because of syntactic reasons. What's your plan for them?

Cheers,

-- 
Julien Grall