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

[Julien Grall <julien@xxxxxxx>]

Hi Luca,

On 05/07/2021 11:51, Luca Fancellu wrote:
> Modification to include/public/grant_table.h:
>
> 1) Add doxygen tags to:
>   - Create Grant tables section
>   - include variables in the generated documentation
>   - Used @keepindent/@endkeepindent to enclose comment
>     section that are indented using spaces, to keep
>     the indentation.
> 2) Add .rst file for grant table
>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
> v7 changes:
> - commit message changed
> - Add comment about grant table queries and uses
> to the documentation
> v6 changes:
> - Fix misaligned comment
> - Moved comments to make them display in the docs
> - Included more documentation in the docs
> v5 changes:
> - Move GNTCOPY_* define next to the flags field
> v4 changes:
> - Used @keepindent/@endkeepindent doxygen commands
>    to keep text with spaces indentation.
> - drop changes to grant_entry_v1 comment, it will
>    be changed and included in the docs in a future patch
> - Move docs .rst to "common" folder
> v3 changes:
> - removed tags to skip anonymous union/struct
> - moved back comment pointed out by Jan
> - moved down defines related to struct gnttab_copy
>    as pointed out by Jan
> v2 changes:
> - Revert back to anonymous union/struct
> - add doxygen tags to skip anonymous union/struct
> ---
>   docs/hypercall-interfaces/arm64.rst           |   1 +
>   .../common/grant_tables.rst                   |   9 +
>   docs/xen-doxygen/doxy_input.list              |   1 +
>   xen/include/public/grant_table.h              | 459 +++++++++++-------
>   4 files changed, 288 insertions(+), 182 deletions(-)
>   create mode 100644 docs/hypercall-interfaces/common/grant_tables.rst
>
> diff --git a/docs/hypercall-interfaces/arm64.rst 
> b/docs/hypercall-interfaces/arm64.rst
> index 5e701a2adc..cb4c0d13de 100644
> --- a/docs/hypercall-interfaces/arm64.rst
> +++ b/docs/hypercall-interfaces/arm64.rst
> @@ -8,6 +8,7 @@ Starting points
>   .. toctree::
>      :maxdepth: 2
>   
> +   common/grant_tables
>   
>   
>   Functions
> diff --git a/docs/hypercall-interfaces/common/grant_tables.rst 
> b/docs/hypercall-interfaces/common/grant_tables.rst
> new file mode 100644
> index 0000000000..b8a1ef8759
> --- /dev/null
> +++ b/docs/hypercall-interfaces/common/grant_tables.rst
> @@ -0,0 +1,9 @@
> +.. SPDX-License-Identifier: CC-BY-4.0
> +
> +Grant Tables
> +============
> +
> +.. doxygengroup:: grant_table
> +   :project: Xen
> +   :members:
> +   :undoc-members:
> diff --git a/docs/xen-doxygen/doxy_input.list b/docs/xen-doxygen/doxy_input.list
> index e69de29bb2..233d692fa7 100644
> --- a/docs/xen-doxygen/doxy_input.list
> +++ b/docs/xen-doxygen/doxy_input.list
> @@ -0,0 +1 @@
> +xen/include/public/grant_table.h
> diff --git a/xen/include/public/grant_table.h b/xen/include/public/grant_table.h
> index 84b1d26b36..2f826c952d 100644
> --- a/xen/include/public/grant_table.h
> +++ b/xen/include/public/grant_table.h
> @@ -25,15 +25,19 @@
>    * Copyright (c) 2004, K A Fraser
>    */
>   
> +/**
> + * @file
> + * @brief Interface for granting foreign access to page frames, and receiving
> + * page-ownership transfers.
> + */
> +
>   #ifndef __XEN_PUBLIC_GRANT_TABLE_H__
>   #define __XEN_PUBLIC_GRANT_TABLE_H__
>   
>   #include "xen.h"
>   
> -/*
> - * `incontents 150 gnttab Grant Tables
`incontents is used by the script xen-headers to generate I believe [1].

Looking through the commit messages, I can't find any suggestion that 
the existing documentation has been retired or else. So can you clarify 
what's the intention?
If the plan to move to doxygen, then I think that
  1) the commit message or cover letter ought to explain why this is 
better than the current documentation
  2) you should remove xen-headers or outline the plan to do that. Note 
that after this series, I believe the bits for the grant table would end 
up to be broken.
Cheers,

[1] https://xenbits.xen.org/docs/unstable/hypercall/index.html

--
Julien Grall