[05/14] kernel-doc: public/features.h
diff mbox series

Message ID 20200806234933.16448-5-sstabellini@kernel.org
State New
Headers show
Series
  • [01/14] kernel-doc: public/arch-arm.h
Related show

Commit Message

Stefano Stabellini Aug. 6, 2020, 11:49 p.m. UTC
From: Stefano Stabellini <stefano.stabellini@xilinx.com>

Convert in-code comments to kernel-doc format wherever possible.

Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
---
 xen/include/public/features.h | 78 ++++++++++++++++++++++++++---------
 1 file changed, 59 insertions(+), 19 deletions(-)

Comments

Jan Beulich Aug. 7, 2020, 12:54 p.m. UTC | #1
On 07.08.2020 01:49, Stefano Stabellini wrote:
> @@ -41,19 +41,25 @@
>   * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
>   */
>  
> -/*
> - * If set, the guest does not need to write-protect its pagetables, and can
> - * update them via direct writes.
> +/**
> + * DOC: XENFEAT_writable_page_tables
> + *
> + * If set, the guest does not need to write-protect its pagetables, and
> + * can update them via direct writes.
>   */
>  #define XENFEAT_writable_page_tables       0

I dislike such redundancy (and it's more noticable here than with
the struct-s). Is there really no way for the tool to find the
right item, the more that in the cover letter you say that you
even need to get the placement right, i.e. there can't be e.g.
intervening #define-s?

Jan
Stefano Stabellini Aug. 7, 2020, 9:52 p.m. UTC | #2
On Fri, 7 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 01:49, Stefano Stabellini wrote:
> > @@ -41,19 +41,25 @@
> >   * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
> >   */
> >  
> > -/*
> > - * If set, the guest does not need to write-protect its pagetables, and can
> > - * update them via direct writes.
> > +/**
> > + * DOC: XENFEAT_writable_page_tables
> > + *
> > + * If set, the guest does not need to write-protect its pagetables, and
> > + * can update them via direct writes.
> >   */
> >  #define XENFEAT_writable_page_tables       0
> 
> I dislike such redundancy (and it's more noticable here than with
> the struct-s). Is there really no way for the tool to find the
> right item, the more that in the cover letter you say that you
> even need to get the placement right, i.e. there can't be e.g.
> intervening #define-s?

Let me clarify that the right placement (nothing between the comment and
the following structure) is important for structs, typedefs, etc., but
not for "DOC". DOC is freeform and doesn't have to be followed by
anything specifically.


In regards to the redundancy, there is only another option, that I
didn't choose because it leads to worse documents being generated.
However, they are still readable, so if the agreement is to use the
other format, I would be OK with it.


The other format is the keyword "macro" (this one would have to have the
right placement, straight on top of the #define):

/**
 * macro XENFEAT_writable_page_tables
 *
 * If set, the guest does not need to write-protect its pagetables, and
 * can update them via direct writes.
 */


Which could be further simplified to:

/**
 * macro
 *
 * If set, the guest does not need to write-protect its pagetables, and
 * can update them via direct writes.
 */


In terms of redundancy, that's the best we can do.

The reason why I say it is not optimal is that with DOC the pleudo-html
generated via sphinx is:

---
* XENFEAT_writable_page_tables *

If set, the guest does not need to write-protect its pagetables, and
can update them via direct writes.
---

While with macro, two () parenthesis gets added to the title, and also an
empty "Parameters" section gets added, like this:

---
* XENFEAT_writable_page_tables() *

** Parameters **

** Description **

If set, the guest does not need to write-protect its pagetables, and
can update them via direct writes.
---


I think it could be confusing to the user: it looks like a macro with
parameters, which is not what we want.

For that reason, I think we should stick with "DOC" for now.
Jan Beulich Aug. 17, 2020, 3:26 p.m. UTC | #3
On 07.08.2020 23:52, Stefano Stabellini wrote:
> On Fri, 7 Aug 2020, Jan Beulich wrote:
>> On 07.08.2020 01:49, Stefano Stabellini wrote:
>>> @@ -41,19 +41,25 @@
>>>   * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
>>>   */
>>>  
>>> -/*
>>> - * If set, the guest does not need to write-protect its pagetables, and can
>>> - * update them via direct writes.
>>> +/**
>>> + * DOC: XENFEAT_writable_page_tables
>>> + *
>>> + * If set, the guest does not need to write-protect its pagetables, and
>>> + * can update them via direct writes.
>>>   */
>>>  #define XENFEAT_writable_page_tables       0
>>
>> I dislike such redundancy (and it's more noticable here than with
>> the struct-s). Is there really no way for the tool to find the
>> right item, the more that in the cover letter you say that you
>> even need to get the placement right, i.e. there can't be e.g.
>> intervening #define-s?
> 
> Let me clarify that the right placement (nothing between the comment and
> the following structure) is important for structs, typedefs, etc., but
> not for "DOC". DOC is freeform and doesn't have to be followed by
> anything specifically.
> 
> 
> In regards to the redundancy, there is only another option, that I
> didn't choose because it leads to worse documents being generated.
> However, they are still readable, so if the agreement is to use the
> other format, I would be OK with it.
> 
> 
> The other format is the keyword "macro" (this one would have to have the
> right placement, straight on top of the #define):
> 
> /**
>  * macro XENFEAT_writable_page_tables
>  *
>  * If set, the guest does not need to write-protect its pagetables, and
>  * can update them via direct writes.
>  */
> 
> 
> Which could be further simplified to:
> 
> /**
>  * macro
>  *
>  * If set, the guest does not need to write-protect its pagetables, and
>  * can update them via direct writes.
>  */
> 
> 
> In terms of redundancy, that's the best we can do.
> 
> The reason why I say it is not optimal is that with DOC the pleudo-html
> generated via sphinx is:
> 
> ---
> * XENFEAT_writable_page_tables *
> 
> If set, the guest does not need to write-protect its pagetables, and
> can update them via direct writes.
> ---
> 
> While with macro, two () parenthesis gets added to the title, and also an
> empty "Parameters" section gets added, like this:
> 
> ---
> * XENFEAT_writable_page_tables() *
> 
> ** Parameters **
> 
> ** Description **
> 
> If set, the guest does not need to write-protect its pagetables, and
> can update them via direct writes.
> ---
> 
> 
> I think it could be confusing to the user: it looks like a macro with
> parameters, which is not what we want.

Agreed, so ...

> For that reason, I think we should stick with "DOC" for now.

... if there are no (better) alternatives we'll have to live with the
redundancy.

Jan
Stefano Stabellini Aug. 17, 2020, 10:56 p.m. UTC | #4
On Mon, 17 Aug 2020, Jan Beulich wrote:
> On 07.08.2020 23:52, Stefano Stabellini wrote:
> > On Fri, 7 Aug 2020, Jan Beulich wrote:
> >> On 07.08.2020 01:49, Stefano Stabellini wrote:
> >>> @@ -41,19 +41,25 @@
> >>>   * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
> >>>   */
> >>>  
> >>> -/*
> >>> - * If set, the guest does not need to write-protect its pagetables, and can
> >>> - * update them via direct writes.
> >>> +/**
> >>> + * DOC: XENFEAT_writable_page_tables
> >>> + *
> >>> + * If set, the guest does not need to write-protect its pagetables, and
> >>> + * can update them via direct writes.
> >>>   */
> >>>  #define XENFEAT_writable_page_tables       0
> >>
> >> I dislike such redundancy (and it's more noticable here than with
> >> the struct-s). Is there really no way for the tool to find the
> >> right item, the more that in the cover letter you say that you
> >> even need to get the placement right, i.e. there can't be e.g.
> >> intervening #define-s?
> > 
> > Let me clarify that the right placement (nothing between the comment and
> > the following structure) is important for structs, typedefs, etc., but
> > not for "DOC". DOC is freeform and doesn't have to be followed by
> > anything specifically.
> > 
> > 
> > In regards to the redundancy, there is only another option, that I
> > didn't choose because it leads to worse documents being generated.
> > However, they are still readable, so if the agreement is to use the
> > other format, I would be OK with it.
> > 
> > 
> > The other format is the keyword "macro" (this one would have to have the
> > right placement, straight on top of the #define):
> > 
> > /**
> >  * macro XENFEAT_writable_page_tables
> >  *
> >  * If set, the guest does not need to write-protect its pagetables, and
> >  * can update them via direct writes.
> >  */
> > 
> > 
> > Which could be further simplified to:
> > 
> > /**
> >  * macro
> >  *
> >  * If set, the guest does not need to write-protect its pagetables, and
> >  * can update them via direct writes.
> >  */
> > 
> > 
> > In terms of redundancy, that's the best we can do.
> > 
> > The reason why I say it is not optimal is that with DOC the pleudo-html
> > generated via sphinx is:
> > 
> > ---
> > * XENFEAT_writable_page_tables *
> > 
> > If set, the guest does not need to write-protect its pagetables, and
> > can update them via direct writes.
> > ---
> > 
> > While with macro, two () parenthesis gets added to the title, and also an
> > empty "Parameters" section gets added, like this:
> > 
> > ---
> > * XENFEAT_writable_page_tables() *
> > 
> > ** Parameters **
> > 
> > ** Description **
> > 
> > If set, the guest does not need to write-protect its pagetables, and
> > can update them via direct writes.
> > ---
> > 
> > 
> > I think it could be confusing to the user: it looks like a macro with
> > parameters, which is not what we want.
> 
> Agreed, so ...
> 
> > For that reason, I think we should stick with "DOC" for now.
> 
> ... if there are no (better) alternatives we'll have to live with the
> redundancy.

Thanks Jan. I would prefer to get this series in as is (with the other
minor changes we discussed) as basic enablement for kernel-doc. I
volunteer to have a look into this issue and try to come up with a
better alternative afterward.

Patch
diff mbox series

diff --git a/xen/include/public/features.h b/xen/include/public/features.h
index 1613b2aab8..524d1758c4 100644
--- a/xen/include/public/features.h
+++ b/xen/include/public/features.h
@@ -27,8 +27,8 @@ 
 #ifndef __XEN_PUBLIC_FEATURES_H__
 #define __XEN_PUBLIC_FEATURES_H__
 
-/*
- * `incontents 200 elfnotes_features XEN_ELFNOTE_FEATURES
+/**
+ * DOC: XEN_ELFNOTE_FEATURES
  *
  * The list of all the features the guest supports. They are set by
  * parsing the XEN_ELFNOTE_FEATURES and XEN_ELFNOTE_SUPPORTED_FEATURES
@@ -41,19 +41,25 @@ 
  * XENFEAT_dom0 MUST be set if the guest is to be booted as dom0,
  */
 
-/*
- * If set, the guest does not need to write-protect its pagetables, and can
- * update them via direct writes.
+/**
+ * DOC: XENFEAT_writable_page_tables
+ *
+ * If set, the guest does not need to write-protect its pagetables, and
+ * can update them via direct writes.
  */
 #define XENFEAT_writable_page_tables       0
 
-/*
+/**
+ * DOC: XENFEAT_writable_descriptor_tables
+ *
  * If set, the guest does not need to write-protect its segment descriptor
  * tables, and can update them via direct writes.
  */
 #define XENFEAT_writable_descriptor_tables 1
 
-/*
+/**
+ * DOC: XENFEAT_auto_translated_physmap
+ *
  * If set, translation between the guest's 'pseudo-physical' address space
  * and the host's machine address space are handled by the hypervisor. In this
  * mode the guest does not need to perform phys-to/from-machine translations
@@ -61,37 +67,63 @@ 
  */
 #define XENFEAT_auto_translated_physmap    2
 
-/* If set, the guest is running in supervisor mode (e.g., x86 ring 0). */
+/**
+ * DOC: XENFEAT_supervisor_mode_kernel
+ *
+ * If set, the guest is running in supervisor mode (e.g., x86 ring 0).
+ */
 #define XENFEAT_supervisor_mode_kernel     3
 
-/*
+/**
+ * DOC: XENFEAT_pae_pgdir_above_4gb
+ *
  * If set, the guest does not need to allocate x86 PAE page directories
  * below 4GB. This flag is usually implied by auto_translated_physmap.
  */
 #define XENFEAT_pae_pgdir_above_4gb        4
 
-/* x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall? */
+/**
+ * DOC: XENFEAT_mmu_pt_update_preserve_ad
+ * x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall?
+ */
 #define XENFEAT_mmu_pt_update_preserve_ad  5
 
-/* x86: Does this Xen host support the MMU_{CLEAR,COPY}_PAGE hypercall? */
+/**
+ * DOC: XENFEAT_highmem_assist
+ * x86: Does this Xen host support the MMU_{CLEAR,COPY}_PAGE hypercall?
+ */
 #define XENFEAT_highmem_assist             6
 
-/*
+/**
+ * DOC: XENFEAT_gnttab_map_avail_bits
+ *
  * If set, GNTTABOP_map_grant_ref honors flags to be placed into guest kernel
  * available pte bits.
  */
 #define XENFEAT_gnttab_map_avail_bits      7
 
-/* x86: Does this Xen host support the HVM callback vector type? */
+/**
+ * DOC: XENFEAT_hvm_callback_vector
+ * x86: Does this Xen host support the HVM callback vector type?
+ */
 #define XENFEAT_hvm_callback_vector        8
 
-/* x86: pvclock algorithm is safe to use on HVM */
+/**
+ * DOC: XENFEAT_hvm_safe_pvclock
+ * x86: pvclock algorithm is safe to use on HVM
+ */
 #define XENFEAT_hvm_safe_pvclock           9
 
-/* x86: pirq can be used by HVM guests */
+/**
+ * DOC: XENFEAT_hvm_pirqs
+ * x86: pirq can be used by HVM guests
+ */
 #define XENFEAT_hvm_pirqs                 10
 
-/* operation as Dom0 is supported */
+/**
+ * DOC: XENFEAT_dom0
+ * operation as Dom0 is supported
+ */
 #define XENFEAT_dom0                      11
 
 /* Xen also maps grant references at pfn = mfn.
@@ -99,13 +131,21 @@ 
 #define XENFEAT_grant_map_identity        12
  */
 
-/* Guest can use XENMEMF_vnode to specify virtual node for memory op. */
+/**
+ * DOC: XENFEAT_memory_op_vnode_supported
+ * Guest can use XENMEMF_vnode to specify virtual node for memory op.
+ */
 #define XENFEAT_memory_op_vnode_supported 13
 
-/* arm: Hypervisor supports ARM SMC calling convention. */
+/**
+ * DOC: XENFEAT_ARM_SMCCC_supported
+ * arm: Hypervisor supports ARM SMC calling convention.
+ */
 #define XENFEAT_ARM_SMCCC_supported       14
 
-/*
+/**
+ * DOC: XENFEAT_linux_rsdp_unrestricted
+ *
  * x86/PVH: If set, ACPI RSDP can be placed at any address. Otherwise RSDP
  * must be located in lower 1MB, as required by ACPI Specification for IA-PC
  * systems.