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

[Julien Grall <julien@xxxxxxx>]

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@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;
> > };
> 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