Re: [PATCH v6 11/16] scripts: kernel-doc: validate kernel-doc markup with the actual names
Em Mon, 18 Jan 2021 13:35:45 -0700 Jonathan Corbet escreveu: > On Thu, 14 Jan 2021 09:04:47 +0100 > Mauro Carvalho Chehab wrote: > > > Kernel-doc currently expects that the kernel-doc markup to come > > just before the function/enum/struct/union/typedef prototype. > > > > Yet, if it find things like: > > > > /** > > * refcount_add - add a value to a refcount > > * @i: the value to add to the refcount > > * @r: the refcount > > */ > > static inline void __refcount_add(int i, refcount_t *r, int *oldp); > > static inline void refcount_add(int i, refcount_t *r); > > > > Kernel-doc will do the wrong thing: > > > > foobar.h:6: warning: Function parameter or member 'oldp' not described > > in '__refcount_add' > > .. c:function:: void __refcount_add (int i, refcount_t *r, int *oldp) > > > >add a value to a refcount > > > > **Parameters** > > > > ``int i`` > > the value to add to the refcount > > > > ``refcount_t *r`` > > the refcount > > > > ``int *oldp`` > > *undescribed* > > > > Basically, it will document "__refcount_add" with the kernel-doc > > markup for refcount_add. > > > > If both functions have the same arguments, this won't even > > produce any warning! > > > > Add a logic to check if the kernel-doc identifier matches the actual > > name of the C function or data structure that will be documented. > > > > Signed-off-by: Mauro Carvalho Chehab > > I've applied this one; Thanks! > it seems useful to have even if it creates more > warnings that Stephen will duly email me about tomorrow...:) I have parts > 1-10 set aside and will apply any that don't get picked up directly by the > maintainers involved. Yeah, new warnings are unavoidable, as new patches may be introducing extra issues. Hopefully, the new warning will help people to detect the issue earlier before submitting upstream. Thanks, Mauro
Re: [PATCH v6 11/16] scripts: kernel-doc: validate kernel-doc markup with the actual names
On Thu, 14 Jan 2021 09:04:47 +0100 Mauro Carvalho Chehab wrote: > Kernel-doc currently expects that the kernel-doc markup to come > just before the function/enum/struct/union/typedef prototype. > > Yet, if it find things like: > > /** >* refcount_add - add a value to a refcount >* @i: the value to add to the refcount >* @r: the refcount >*/ > static inline void __refcount_add(int i, refcount_t *r, int *oldp); > static inline void refcount_add(int i, refcount_t *r); > > Kernel-doc will do the wrong thing: > > foobar.h:6: warning: Function parameter or member 'oldp' not described > in '__refcount_add' > .. c:function:: void __refcount_add (int i, refcount_t *r, int *oldp) > > add a value to a refcount > > **Parameters** > > ``int i`` > the value to add to the refcount > > ``refcount_t *r`` > the refcount > > ``int *oldp`` > *undescribed* > > Basically, it will document "__refcount_add" with the kernel-doc > markup for refcount_add. > > If both functions have the same arguments, this won't even > produce any warning! > > Add a logic to check if the kernel-doc identifier matches the actual > name of the C function or data structure that will be documented. > > Signed-off-by: Mauro Carvalho Chehab I've applied this one; it seems useful to have even if it creates more warnings that Stephen will duly email me about tomorrow...:) I have parts 1-10 set aside and will apply any that don't get picked up directly by the maintainers involved. Thanks, jon
Re: [PATCH v6 11/16] scripts: kernel-doc: validate kernel-doc markup with the actual names
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on next-20210113] [also build test WARNING on v5.11-rc3] [cannot apply to lwn/docs-next kees/for-next/pstore mac80211-next/master mac80211/master pza/reset/next linus/master v5.11-rc3 v5.11-rc2 v5.11-rc1] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Fix-several-bad-kernel-doc-markups/20210114-161010 base:aa515cdce7a151dcc14b7600d33f1414c6fa32c9 config: mips-nlm_xlp_defconfig (attached as .config) compiler: mips64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/1e9159b3640012fc5fab8508b1c5635ac13a55e9 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Mauro-Carvalho-Chehab/Fix-several-bad-kernel-doc-markups/20210114-161010 git checkout 1e9159b3640012fc5fab8508b1c5635ac13a55e9 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=mips If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): arch/mips/kernel/module.c:289: warning: Function parameter or member 'reloc_handler' not described in 'int' >> arch/mips/kernel/module.c:289: warning: expecting prototype for >> reloc_handler(). Prototype was for int() instead -- arch/mips/kernel/kprobes.c:196: warning: Function parameter or member 'p' not described in 'evaluate_branch_instruction' arch/mips/kernel/kprobes.c:196: warning: Function parameter or member 'regs' not described in 'evaluate_branch_instruction' arch/mips/kernel/kprobes.c:196: warning: Function parameter or member 'kcb' not described in 'evaluate_branch_instruction' >> arch/mips/kernel/kprobes.c:196: warning: expecting prototype for >> evaluate_branch_instrucion(). Prototype was for >> evaluate_branch_instruction() instead vim +289 arch/mips/kernel/module.c 5d3c792583b60406 Paul Burton 2016-02-04 272 430d0b88943afffd Paul Burton 2017-03-30 273 /** 430d0b88943afffd Paul Burton 2017-03-30 274 * reloc_handler() - Apply a particular relocation to a module 430d0b88943afffd Paul Burton 2017-03-30 275 * @me: the module to apply the reloc to 430d0b88943afffd Paul Burton 2017-03-30 276 * @location: the address at which the reloc is to be applied 430d0b88943afffd Paul Burton 2017-03-30 277 * @base: the existing value at location for REL-style; 0 for RELA-style 430d0b88943afffd Paul Burton 2017-03-30 278 * @v: the value of the reloc, with addend for RELA-style 430d0b88943afffd Paul Burton 2017-03-30 279 * 430d0b88943afffd Paul Burton 2017-03-30 280 * Each implemented reloc_handler function applies a particular type of 430d0b88943afffd Paul Burton 2017-03-30 281 * relocation to the module @me. Relocs that may be found in either REL or RELA 430d0b88943afffd Paul Burton 2017-03-30 282 * variants can be handled by making use of the @base & @v parameters which are 430d0b88943afffd Paul Burton 2017-03-30 283 * set to values which abstract the difference away from the particular reloc 430d0b88943afffd Paul Burton 2017-03-30 284 * implementations. 430d0b88943afffd Paul Burton 2017-03-30 285 * 430d0b88943afffd Paul Burton 2017-03-30 286 * Return: 0 upon success, else -ERRNO 430d0b88943afffd Paul Burton 2017-03-30 287 */ 430d0b88943afffd Paul Burton 2017-03-30 288 typedef int (*reloc_handler)(struct module *me, u32 *location, 430d0b88943afffd Paul Burton 2017-03-30 @289 u32 base, Elf_Addr v, bool rela); 430d0b88943afffd Paul Burton 2017-03-30 290 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH v6 11/16] scripts: kernel-doc: validate kernel-doc markup with the actual names
Hi Mauro,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20210113]
[also build test WARNING on v5.11-rc3]
[cannot apply to lwn/docs-next kees/for-next/pstore mac80211-next/master
mac80211/master pza/reset/next linus/master v5.11-rc3 v5.11-rc2 v5.11-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Fix-several-bad-kernel-doc-markups/20210114-161010
base:aa515cdce7a151dcc14b7600d33f1414c6fa32c9
config: arm64-alldefconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/1e9159b3640012fc5fab8508b1c5635ac13a55e9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Mauro-Carvalho-Chehab/Fix-several-bad-kernel-doc-markups/20210114-161010
git checkout 1e9159b3640012fc5fab8508b1c5635ac13a55e9
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
kernel/seccomp.c:126: warning: Function parameter or member 'list' not
described in 'seccomp_kaddfd'
kernel/seccomp.c:390: warning: Function parameter or member 'ret' not
described in 'ACTION_ONLY'
>> kernel/seccomp.c:390: warning: expecting prototype for
>> seccomp_run_filters(). Prototype was for ACTION_ONLY() instead
kernel/seccomp.c:553: warning: Function parameter or member 'tsk' not
described in 'seccomp_filter_release'
kernel/seccomp.c:573: warning: Function parameter or member 'flags' not
described in 'seccomp_sync_threads'
--
drivers/base/platform-msi.c:336: warning: Function parameter or member
'is_tree' not described in '__platform_msi_create_device_domain'
>> drivers/base/platform-msi.c:336: warning: expecting prototype for
>> platform_msi_create_device_domain(). Prototype was for
>> __platform_msi_create_device_domain() instead
--
>> drivers/iommu/iommu.c:956: warning: expecting prototype for
>> iommu_group_for_each_dev(). Prototype was for __iommu_group_for_each_dev()
>> instead
drivers/iommu/iommu.c:2976: warning: Function parameter or member 'drvdata'
not described in 'iommu_sva_bind_device'
--
>> kernel/irq/msi.c:534: warning: expecting prototype for
>> __msi_domain_free_irqs(). Prototype was for msi_domain_free_irqs() instead
--
>> kernel/irq/ipi.c:264: warning: expecting prototype for ipi_send_mask().
>> Prototype was for __ipi_send_mask() instead
vim +390 kernel/seccomp.c
f9d480b6ffbeb336 YiFei Zhu 2020-10-11 380
e2cfabdfd0756482 Will Drewry2012-04-12 381 /**
285fdfc5d9959a21 Mickaël Salaün 2016-09-20 382 * seccomp_run_filters -
evaluates all seccomp filters against @sd
285fdfc5d9959a21 Mickaël Salaün 2016-09-20 383 * @sd: optional seccomp
data to be passed to filters
deb4de8b31bc5bf2 Kees Cook 2017-08-02 384 * @match: stores struct
seccomp_filter that resulted in the return value,
deb4de8b31bc5bf2 Kees Cook 2017-08-02 385 * unless filter
returned SECCOMP_RET_ALLOW, in which case it will
deb4de8b31bc5bf2 Kees Cook 2017-08-02 386 * be unchanged.
e2cfabdfd0756482 Will Drewry2012-04-12 387 *
e2cfabdfd0756482 Will Drewry2012-04-12 388 * Returns valid seccomp
BPF response codes.
e2cfabdfd0756482 Will Drewry2012-04-12 389 */
0466bdb99e8744bc Kees Cook 2017-08-11 @390 #define ACTION_ONLY(ret)
((s32)((ret) & (SECCOMP_RET_ACTION_FULL)))
deb4de8b31bc5bf2 Kees Cook 2017-08-02 391 static u32
seccomp_run_filters(const struct seccomp_data *sd,
deb4de8b31bc5bf2 Kees Cook 2017-08-02 392
struct seccomp_filter **match)
e2cfabdfd0756482 Will Drewry2012-04-12 393 {
acf3b2c71ed20c53 Will Drewry2012-04-12 394 u32 ret =
SECCOMP_RET_ALLOW;
8225d3853f34f6cf Pranith Kumar 2014-11-21 395 /* Make sure
cross-thread synced filter points somewhere sane. */
8225d3853f34f6cf Pranith Kumar 2014-11-21 396 struct seccomp_filter
*f =
506458efaf153c1e Will Deacon2017-10-24 397
READ_ONCE(current->seccomp.filter);
acf3b2c71ed20c53 Will Drewry2012-04-12 398
acf3b2c71ed20c53 Will Drewry2012-04-12 399 /* Ensure unexpected
behavior doesn't result in failing open. */
0d42d73a37ff9102 Igor Stoppa2018-09-05 400 if (WARN_ON(f == NULL))
4d3b0b05aae9ee9c Kees Cook 2017-08-11 401 retur
[PATCH v6 11/16] scripts: kernel-doc: validate kernel-doc markup with the actual names
Kernel-doc currently expects that the kernel-doc markup to come
just before the function/enum/struct/union/typedef prototype.
Yet, if it find things like:
/**
* refcount_add - add a value to a refcount
* @i: the value to add to the refcount
* @r: the refcount
*/
static inline void __refcount_add(int i, refcount_t *r, int *oldp);
static inline void refcount_add(int i, refcount_t *r);
Kernel-doc will do the wrong thing:
foobar.h:6: warning: Function parameter or member 'oldp' not described
in '__refcount_add'
.. c:function:: void __refcount_add (int i, refcount_t *r, int *oldp)
add a value to a refcount
**Parameters**
``int i``
the value to add to the refcount
``refcount_t *r``
the refcount
``int *oldp``
*undescribed*
Basically, it will document "__refcount_add" with the kernel-doc
markup for refcount_add.
If both functions have the same arguments, this won't even
produce any warning!
Add a logic to check if the kernel-doc identifier matches the actual
name of the C function or data structure that will be documented.
Signed-off-by: Mauro Carvalho Chehab
---
scripts/kernel-doc | 62 ++
1 file changed, 46 insertions(+), 16 deletions(-)
diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 6325bec3f66f..a9a92e623dbc 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -382,6 +382,9 @@ my $inline_doc_state;
# 'function', 'struct', 'union', 'enum', 'typedef'
my $decl_type;
+# Name of the kernel-doc identifier for non-DOC markups
+my $identifier;
+
my $doc_start = '^/\*\*\s*$'; # Allow whitespace at end of comment start.
my $doc_end = '\*/';
my $doc_com = '\s*\*\s*';
@@ -1203,6 +1206,11 @@ sub dump_struct($$) {
$declaration_name = $2;
my $members = $3;
+ if ($identifier ne $declaration_name) {
+ print STDERR "${file}:$.: warning: expecting prototype for
$decl_type $identifier. Prototype was for $decl_type $declaration_name
instead\n";
+ return;
+ }
+
# ignore members marked private:
$members =~ s/\/\*\s*private:.*?\/\*\s*public:.*?\*\///gosi;
$members =~ s/\/\*\s*private:.*//gosi;
@@ -1391,6 +1399,11 @@ sub dump_enum($$) {
}
if ($members) {
+ if ($identifier ne $declaration_name) {
+ print STDERR "${file}:$.: warning: expecting prototype for enum
$identifier. Prototype was for enum $declaration_name instead\n";
+ return;
+ }
+
my %_members;
$members =~ s/\s+$//;
@@ -1451,6 +1464,11 @@ sub dump_typedef($$) {
my $args = $3;
$return_type =~ s/^\s+//;
+ if ($identifier ne $declaration_name) {
+ print STDERR "${file}:$.: warning: expecting prototype for typedef
$identifier. Prototype was for typedef $declaration_name instead\n";
+ return;
+ }
+
create_parameterlist($args, ',', $file, $declaration_name);
output_declaration($declaration_name,
@@ -1477,6 +1495,11 @@ sub dump_typedef($$) {
if ($x =~ /typedef.*\s+(\w+)\s*;/) {
$declaration_name = $1;
+ if ($identifier ne $declaration_name) {
+ print STDERR "${file}:$.: warning: expecting prototype for typedef
$identifier. Prototype was for typedef $declaration_name instead\n";
+ return;
+ }
+
output_declaration($declaration_name,
'typedef',
{'typedef' => $declaration_name,
@@ -1796,6 +1819,11 @@ sub dump_function($$) {
return;
}
+if ($identifier ne $declaration_name) {
+ print STDERR "${file}:$.: warning: expecting prototype for
$identifier(). Prototype was for $declaration_name() instead\n";
+ return;
+}
+
my $prms = join " ", @parameterlist;
check_sections($file, $declaration_name, "function", $sectcheck, $prms);
@@ -1878,6 +1906,7 @@ sub tracepoint_munge($) {
"$prototype\n";
} else {
$prototype = "static inline void
trace_$tracepointname($tracepointargs)";
+ $identifier = "trace_$identifier";
}
}
@@ -2041,7 +2070,6 @@ sub process_normal() {
#
sub process_name($$) {
my $file = shift;
-my $identifier;
my $descr;
if (/$doc_block/o) {
@@ -2054,12 +2082,19 @@ sub process_name($$) {
} else {
$section = $1;
}
-}
-elsif (/$doc_decl/o) {
+} elsif (/$doc_decl/o) {
$identifier = $1;
- if (/\s*([\w\s]+?)(\(\))?\s*-/) {
+ if (/\s*([\w\s]+?)(\(\))?\s*([-:].*)?$/) {
$identifier = $1;
}
+ if ($identifier =~ m/^(struct|union|enum|typedef)\b\s*(\S*)/) {
+ $decl_type = $1;
+ $identifier = $2;
+ } else {
+ $decl_type = 'function';
+ $identifier =~ s/^define\s+//;
+ }
+
