scripts/kernel-doc parsing issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[reduced Cc: list]

[was: Re: [PATCH 1/1] kernel-doc: Support arrays of pointers struct fields]

On 2/5/24 16:05, Jonathan Corbet wrote:

> Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> writes:
> 
>>> Sigh ... seeing more indecipherable regexes added to kernel-doc is like
>>> seeing another load of plastic bags dumped into the ocean...  it doesn't
>>> change the basic situation, but it's still sad.
>>>
>>> Oh well, applied, thanks.
>>
>> Thanks. I have to say I feel the same...
>>
>> Regexes aren't great for parsing C, that's for sure. :-I But what are the
>> options? Write a proper parser for (a subset of) C?
> 
> Every now and then I've pondered on this a bit.  There are parsers out
> there, of course; we could consider using something like tree-sitter.
> There's just two little problems:
> 
> - That's a massive dependency to drag into the docs build that seems
>   unlikely to speed things up.
> 
> - kernel-doc is really two parsers - one for C code, one for the
>   comment syntax.  Strangely, nobody has written a grammar for this
>   combination.
> 
> A suitably motivated developer could probably create a C+kerneldoc
> grammer that would let us make a rock-solid, tree-sitter-based parser
> that would be mostly maintained by somebody else.  But that doesn't get
> us around the "adding a big dependency" problem.
> 
> <back to work now...>

As I said here on the RFC patch from Sakari:
https://lore.kernel.org/all/aa94772b-7010-4bba-b099-d3b8fe1b97aa@xxxxxxxxxxxxx/

"Yet another kernel-doc bug. I have a list of 5 or 6 or 8 bugs that are
similar to this one, but I didn't have this one."

The patch to report Excess struct or union members has unearthed several
kernel-doc "parsing" problems.

I have not tried to fix any of these in scripts/kernel-doc yet. I might get
around to it, but it's not a high priority for me.


Examples:

1) drivers/slimbus/stream.c:49: warning: Excess struct member 'segdist_codes' description in 'segdist_code'

struct declaration and definition together. Also possible that the leading "static const"
confuses scripts/kernel-doc.

2) include/linux/spi/spi.h:246: warning: Function parameter or struct member 'cs_index_mask:SPI_CS_CNT_MAX' not described in 'spi_device'
include/linux/spi/spi.h:246: warning: Excess struct member 'cs_index_mask' description in 'spi_device'

scripts/kernel-doc handles some bit fields in structs successfully, so something is
different about this one.

3) fs/ntfs/compress.c:24: warning: cannot understand function prototype: 'typedef enum '

fs/ntfs/* has been removed in linux-next (still in mainline for a little while), but this
shows that scripts/kernel-doc does not handle a 'typedef enum' successfully.

4) drivers/misc/vmw_balloon.c:260: warning: Excess struct member 'reserved' description in 'vmballoon_batch_entry'

This may be the same problem as #2, with using bit fields in a struct.

5) drivers/base/power/runtime.c:362: warning: Excess function parameter 'dev' description in '__rpm_callback'

Confused by either the first function parameter (a function pointer) or the trailing
__releases() and __acquires() attributes.

6) drivers/md/bcache/request.c:309: warning: expecting prototype for bch_data_insert(). Prototype was for CLOSURE_CALLBACK() instead

and
fs/bcachefs/io_write.c:1558: warning: expecting prototype for bch2_write(). Prototype was for CLOSURE_CALLBACK() instead

CLOSURE_CALLBACK() and function parameters are confusing scripts/kernel-doc.

7) drivers/iio/adc/at91-sama5d2_adc.c:471: warning: Excess struct member 'adc_channels' description in 'at91_adc_platform'

Fixed by Sakari's patch.  :)

8) drivers/pci/controller/pcie-iproc-msi.c:110: warning: Excess struct member 'reg_offsets' description in 'iproc_msi'

Fixed by Sakari's patch.  :)

9) drivers/usb/gadget/udc/pch_udc.c:361: warning: Excess struct member 'stall' description in 'pch_udc_dev'
pch_udc.c:361: warning: Excess struct member 'prot_stall' description in 'pch_udc_dev'
pch_udc.c:361: warning: Excess struct member 'registered' description in 'pch_udc_dev'
pch_udc.c:361: warning: Excess struct member 'suspended' description in 'pch_udc_dev'
pch_udc.c:361: warning: Excess struct member 'connected' description in 'pch_udc_dev'
pch_udc.c:361: warning: Excess struct member 'vbus_session' description in 'pch_udc_dev'
pch_udc.c:361: warning: Excess struct member 'set_cfg_not_acked' description in 'pch_udc_dev'
pch_udc.c:361: warning: Excess struct member 'waiting_zlp_ack' description in 'pch_udc_dev'

All of these except @registered (which is just an Excess description) are declared with one
'unsigned' followed by a list of bit fields, which isn't kernel coding style but it is valid C.
or it might just be 'unsigned' without having a following 'int' that is the problem. I don't
know -- haven't looked yet.

10) Matthew Wilcox pointed out to me that commit 0d55d48b19ff is causing problems with
generated output. A few instances of using TAB or multiple spaces have been patched
recently, but there are others that are not being addressed. I don't have a list of these.


I don't know anything about tree-sitter, so if I were going to add a parser, I would
probably (foolishly?) first try using sparse and a sparse extension.

cheers.
-- 
#Randy




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux