linux-kernel - Re: [PATCH] kernel-doc: Make the description of return value readable

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] [day] [month] [year] [list]

Message-ID: <YA8AZ/pOhts/DneP@kernel.org>
Date:   Mon, 25 Jan 2021 19:31:19 +0200
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc:     corbet@....net,
        Nícolas F. R. A. Prado 
        <nfraprado@...tonmail.com>, Ben Widawsky <ben.widawsky@...el.com>,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kernel-doc: Make the description of return value readable

On Mon, Jan 25, 2021 at 10:53:53AM +0100, Mauro Carvalho Chehab wrote:
> Em Mon, 25 Jan 2021 10:19:04 +0200
> Jarkko Sakkinen <jarkko@...nel.org> escreveu:
> 
> > The description on how to describe return values is over-complicated, and
> > hard to follow. For alien reason, the body of the section is a note, and
> > the first paragraph speaks about 'Return', albeit the section name is
> > actually 'Return:'.
> > 
> > To give a better help when both implementing and reviewing patches, provide
> > a straight-forward guideline, how to decribe return values, instead of
> > providing a note that starts by "blacklisting" one of the infinite possible
> > options of doing it wrong.
> > 
> > This decreases the cumulative amount of time, which is probably
> > substantial, on this otherwise somewhat trivial topic.
> > 
> > Signed-off-by: Jarkko Sakkinen <jarkko@...nel.org>
> > ---
> >  Documentation/doc-guide/kernel-doc.rst | 34 +++++++-------------------
> >  1 file changed, 9 insertions(+), 25 deletions(-)
> > 
> > diff --git a/Documentation/doc-guide/kernel-doc.rst b/Documentation/doc-guide/kernel-doc.rst
> > index 79aaa55d6bcf..dc5e1722c150 100644
> > --- a/Documentation/doc-guide/kernel-doc.rst
> > +++ b/Documentation/doc-guide/kernel-doc.rst
> > @@ -136,34 +136,18 @@ Examples::
> >  Return values
> >  ~~~~~~~~~~~~~
> >  
> > -The return value, if any, should be described in a dedicated section
> > -named ``Return``.
> > +The return value, if any, should be described in a dedicated section named
> > +``Return:``.
> >  
> > -.. note::
> > -
> > -  #) The multi-line descriptive text you provide does *not* recognize
> > -     line breaks, so if you try to format some text nicely, as in::
> > -
> > -	* Return:
> > -	* 0 - OK
> > -	* -EINVAL - invalid argument
> > -	* -ENOMEM - out of memory
> > -
> > -     this will all run together and produce::
> > -
> > -	Return: 0 - OK -EINVAL - invalid argument -ENOMEM - out of memory
> > -
> > -     So, in order to produce the desired line breaks, you need to use a
> > -     ReST list, e. g.::
> > +In order to describe multiple return values, a ReST list should be used. That
> > +way Sphinx knows how to line-up them properly::
> >  
> > -      * Return:
> > -      * * 0		- OK to runtime suspend the device
> > -      * * -EBUSY	- Device should not be runtime suspended
> > +  * Return:
> > +  * - 0:	The run-time is allowed to suspend the device.
> > +  * - -EBUSY:	Device should not be suspended.
> >  
> > -  #) If the descriptive text you provide has lines that begin with
> > -     some phrase followed by a colon, each of those phrases will be taken
> > -     as a new section heading, which probably won't produce the desired
> > -     effect.
> > +Using a dash rather than asterisk an is probably a better idea, because it does
> > +not meddle as much with the C-comments.
> 
> Well, there are different opinions with that regards... Some devs think that
> using:
> 
> 	- -EBUSY
> 
> is confusing. I ended agreeing with that. That's why the recommendation is
> to use an asterisk.

Maybe then whitelisting two options would be the way to move forward?

> Thanks,
> Mauro

/Jarkko