linux-kernel - Re: [PATCH v4] kernel.h: add comments for system

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250907234616.134dbda4@foz.lan>
Date: Sun, 7 Sep 2025 23:46:16 +0200
From: Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: Jani Nikula <jani.nikula@...ux.intel.com>, linux-kernel@...r.kernel.org,
 Andrew Morton <akpm@...ux-foundation.org>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, Pavel Machek <pavel@....cz>, Len Brown
 <len.brown@...el.com>, linux-pm@...r.kernel.org, Jonathan Corbet
 <corbet@....net>, linux-doc@...r.kernel.org, "James E.J. Bottomley"
 <James.Bottomley@...senpartnership.com>
Subject: Re: [PATCH v4] kernel.h: add comments for system_states

Em Sat, 6 Sep 2025 10:13:23 -0700
Randy Dunlap <rdunlap@...radead.org> escreveu:

> On 9/6/25 1:56 AM, Mauro Carvalho Chehab wrote:
> > Em Fri, 5 Sep 2025 15:07:31 -0700
> > Randy Dunlap <rdunlap@...radead.org> escreveu:
> >   
> >> Hi,
> >>
> >> On 9/5/25 6:38 AM, Mauro Carvalho Chehab wrote:  
> >>> On Fri, Sep 05, 2025 at 04:06:31PM +0300, Jani Nikula wrote:    
> >>>> On Fri, 05 Sep 2025, Mauro Carvalho Chehab <mchehab+huawei@...nel.org> wrote:    
> >>>>> Em Fri, 05 Sep 2025 12:02:37 +0300
> >>>>> Jani Nikula <jani.nikula@...ux.intel.com> escreveu:
> >>>>>    
> >>>>>> On Wed, 03 Sep 2025, Randy Dunlap <rdunlap@...radead.org> wrote:    
> >>>>>>> Provide some basic comments about the system_states and what they imply.
> >>>>>>> Also convert the comments to kernel-doc format.
> >>>>>>>
> >>>>>>> However, kernel-doc does not support kernel-doc notation on extern
> >>>>>>> struct/union/typedef/enum/etc. So I made this a DOC: block so that
> >>>>>>> I can use (insert) it into a Documentation (.rst) file and have it
> >>>>>>> look decent.      
> >>>>>>
> >>>>>> The simple workaround is to separate the enum type and the variable:
> >>>>>>
> >>>>>> /**
> >>>>>>  * kernel-doc for the enum
> >>>>>>  */
> >>>>>> enum system_states {
> >>>>>> 	...
> >>>>>> };
> >>>>>>
> >>>>>> /**
> >>>>>>  * kernel-doc for the variable
> >>>>>>  */
> >>>>>> extern enum system_states system_state;
> >>>>>>
> >>>>>> BR,
> >>>>>> Jani.
> >>>>>>    
> >>>>>>>
> >>>>>>> Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
> >>>>>>> Acked-by: Rafael J. Wysocki <rafael@...nel.org> # v1
> >>>>>>> ---    
> >>
> >> [snip]  
> >>>>> If the problem is with "extern" before enum, fixing kdoc be
> >>>>> fairly trivial.    
> >>>>
> >>>> The non-trivial part is deciding whether you're documenting the enum
> >>>> type or the variable. Both are equally valid options.    
> >>>
> >>> True.
> >>>
> >>> I'd say that, if the variable is under EXPORT_SYMBOL macros, it
> >>> should be documented.    
> >>
> >> Do you mean documented with kernel-doc? How do we do that?
> >> I was under the impression that we don't currently have a way to
> >> use kernel-doc for variables (definitions, only for declarations).  
> > 
> > No, but it shouldn't be hard to add something like:
> > 
> > 	/**
> > 	 * global system_state - stores system state
> > 	 * <some description>
> > 	 */
> > 	extern enum system_states system_state;  
> > 
> > and place a dump_global() function at kdoc parser. Ok, one might use
> > DOC:, but IMHO the best is to have a proper handler for it that would
> > automatically pick the type.  
> 
> Nitpick, I would s/global/var/. But I won't complain about "global".
> 
> More importantly, I have seen several patches where people try to document a
> variable, so it seems like something that we should support at some point.

As we're talking about possible new kernel-doc features, there's
something that IMHO we should consider implementing: define
groups. For instance, media videodev2.h uapi has several ones like
this (and this pattern is quite common on other places - either using
BIT() or not):

	/* Values for 'capabilities' field */
	#define V4L2_CAP_VIDEO_CAPTURE      0x00000001  /* Is a video capture device */
	#define V4L2_CAP_VIDEO_OUTPUT       0x00000002  /* Is a video output device */
	#define V4L2_CAP_VIDEO_OVERLAY      0x00000004  /* Can do video overlay */
	#define V4L2_CAP_VBI_CAPTURE        0x00000010  /* Is a raw VBI capture device */
	#define V4L2_CAP_VBI_OUTPUT     0x00000020  /* Is a raw VBI output device */
	#define V4L2_CAP_SLICED_VBI_CAPTURE 0x00000040  /* Is a sliced VBI capture device */
	#define V4L2_CAP_SLICED_VBI_OUTPUT  0x00000080  /* Is a sliced VBI output device */
	#define V4L2_CAP_RDS_CAPTURE        0x00000100  /* RDS data capture */
	#define V4L2_CAP_VIDEO_OUTPUT_OVERLAY   0x00000200  /* Can do video output overlay */
	#define V4L2_CAP_HW_FREQ_SEEK       0x00000400  /* Can do hardware frequency seek  */
	#define V4L2_CAP_RDS_OUTPUT     0x00000800  /* Is an RDS encoder */


We could use something like this:

	/**
	 * defgroup - Values for 'capabilities' field
	 *
	 * @V4L2_CAP_VIDEO_CAPTURE: Is a video capture device
	 * @V4L2_CAP_VIDEO_OUTPUT: Is a video output device
	 * @V4L2_CAP_VIDEO_OVERLAY: Can do video overlay
	 * @V4L2_CAP_VBI_CAPTURE: Is a raw VBI capture device
	 * @V4L2_CAP_VBI_OUTPUT: Is a raw VBI output device
	 * @V4L2_CAP_SLICED_VBI_CAPTURE: Is a sliced VBI capture device
	 * @V4L2_CAP_SLICED_VBI_OUTPUT: Is a sliced VBI output device
	 * @V4L2_CAP_RDS_CAPTURE: RDS data capture
	 * @V4L2_CAP_VIDEO_OUTPUT_OVERLAY: Can do video output overlay
	 * @V4L2_CAP_HW_FREQ_SEEK: Can do hardware frequency seek
	 * @V4L2_CAP_RDS_OUTPUT: Is an RDS encoder
	 */
	#define V4L2_CAP_VIDEO_CAPTURE		0x00000001
	#define V4L2_CAP_VIDEO_OUTPUT		0x00000002
	#define V4L2_CAP_VIDEO_OVERLAY		0x00000004
	#define V4L2_CAP_VBI_CAPTURE		0x00000010
	#define V4L2_CAP_VBI_OUTPUT		0x00000020
	#define V4L2_CAP_SLICED_VBI_CAPTURE	0x00000040
	#define V4L2_CAP_SLICED_VBI_OUTPUT	0x00000080
	#define V4L2_CAP_RDS_CAPTURE		0x00000100
	#define V4L2_CAP_VIDEO_OUTPUT_OVERLAY	0x00000200
	#define V4L2_CAP_HW_FREQ_SEEK		0x00000400
	#define V4L2_CAP_RDS_OUTPUT		0x00000800

I suspect this is easier said than done, but there are lots of
similar stuff at the Kernel. On some cases, it is easier to use
enums, but on others like here, it probably keeps sense to keep
them a defines.

Thanks,
Mauro