On 1/4/21 1:02 am, Jonathan Corbet wrote:

Aditya Srivastava [off-list ref] writes:

quoted

On 29/3/21 7:26 pm, Jonathan Corbet wrote:

quoted

Aditya Srivastava [off-list ref] writes:

quoted

Currently, kernel-doc start parsing the comment as a kernel-doc comment if
it starts with '/**', but does not take into account if the content inside
the comment too, adheres with the expected format.
This results in unexpected and unclear warnings for the user.

E.g., running scripts/kernel-doc -none mm/memcontrol.c emits:
"mm/memcontrol.c:961: warning: expecting prototype for do not fallback to current(). Prototype was for get_mem_cgroup_from_current() instead"

Here kernel-doc parses the corresponding comment as a kernel-doc comment
and expects prototype for it in the next lines, and as a result causing
this warning.

Provide a clearer warning message to the users regarding the same, if the
content inside the comment does not follow the kernel-doc expected format.

Signed-off-by: Aditya Srivastava <redacted>
---
 scripts/kernel-doc | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

This is definitely a capability we want, but I really don't think that
we can turn it on by default - for now.  Experience shows that if you
create a blizzard of warnings, nobody sees any of them.  How many
warnings does this add to a full docs build?

Hi Jonathan, here's the diff I have created for the warnings before
and after the changes:
https://github.com/AdityaSrivast/kernel-tasks/blob/master/random/kernel-doc/kernel_doc_comment_syntax.txt

Around ~1320 new warnings of this type are added to the kernel tree,
and around ~1580 warnings are removed.

So I finally got around to looking at this again...  How did you
generate that file?

I ran scripts/kernel-doc -none on all the files in the kernel tree
before and after appying the changes, and then generated their diff to
find the warnings removed and added.

I tried applying the patch and doing a normal full htmldocs build and
got all of four warnings:

  ./include/linux/seqlock.h:829: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
   * DEFINE_SEQLOCK(sl) - Define a statically allocated seqlock_t
  ./fs/jbd2/journal.c:1391: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
   *  journal_t * jbd2_journal_init_dev() - creates and initialises a journal structure
  ./fs/jbd2/journal.c:1422: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
   *  journal_t * jbd2_journal_init_inode () - creates a journal which maps to a inode.
  ./include/linux/dcache.h:309: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
   *      dget, dget_dlock -      get a reference to a dentry

I think there should be more warnings. For e.g., running kernel-doc
-none ./drivers/usb/mtu3/mtu3.h gives these warnings:

./drivers/usb/mtu3/mtu3.h:75: warning: This comment starts with '/**',
but isn't a kernel-doc comment. Refer
Documentation/doc-guide/kernel-doc.rst
./drivers/usb/mtu3/mtu3.h:86: warning: This comment starts with '/**',
but isn't a kernel-doc comment. Refer
Documentation/doc-guide/kernel-doc.rst
./drivers/usb/mtu3/mtu3.h:143: warning: This comment starts with
'/**', but isn't a kernel-doc comment. Refer
Documentation/doc-guide/kernel-doc.rst

Two observations:

 - This is not an awful lot of warnings - not the blizzard I had
   feared.  At this level, I think we can just merge the patch and
   then, hopefully, fix those cases.

 - All of the warned-about places are *attempts* to write real kerneldoc
   comments, they just got the syntax wrong in one way or another.  It's
   probably not worth the effort to try to detect this case - the
   warning is enough to draw attention to the comment in question.

I agree. Above are some of the cases which are not getting detected by
this patch.
This may be so as I am only allowing the function syntax as mentioned
in the rst file, i.e., "^\s*\*\s*([\w\s]+?)(\(\))?\s*([-:].*)?$" or
("* foo(\(\))? - description")

I probably need to check for pointers as well and other similar case(s).
Maybe I should design a separate check for functions than assigning
$decl_type = 'function' in the first check.

What do you think?

Thanks
Aditya
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees