[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