Re: [cocci] [PATCH] scripts/coccinelle: Add script for using ARRAY_END()

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4925 bytes --]

Hi Markus,

On 2026-03-09T12:17:00+0100, Markus Elfring wrote:
> …
> > This script makes it easy to find more places where that macro should be
> > used.
> 
> See also:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.0-rc3#n94

Thanks!

> …
> > +++ b/scripts/coccinelle/misc/array_end.cocci
> > @@ -0,0 +1,93 @@
> …
> 
> 
> > +// Confidence: ???
> 
> I hope that a more reasonable value can be determined for this information.

I don't know how the scale works.  I know the script has a few false
negatives, and AFAIK there are no false positives.  To what level of
confidence would that belong?

Is that field a keyword, or may I be explicit such as with this?:

// Confidence: no false positives, but a few false negatives

Or maybe I should write that in Comments...

> …
> > +// Comments:
> 
> Please omit such an empty field.

Ok; thanks!

$ grep -rh '^// Comments:' scripts/coccinelle/ | sort | uniq -c
     34 // Comments:
      2 // Comments: -
      1 // Comments: -I ... -all_includes can give more complete results
      1 // Comments: Comments on code can be deleted if near code that is removed.
      1 // Comments: Some false positives on empty default cases in switch statements.
      1 // Comments: requires at least Coccinelle 0.2.4, lex or parse error otherwise
$ find scripts/coccinelle/ -type f | wc -l
76


It seems around half of the existing scripts have that.  You may want to
remove those empty comments.  I added it because the scripts I looked at
do have it.

> …
> > +@i@
> > +@@
> > +
> > +#include <linux/kernel.h>
> 
> I doubt that such an SmPL rule would be required.

Okay, I'll remove it.  Thanks!

> > +
> > +//----------------------------------------------------------
> > +//  For context mode
> > +//----------------------------------------------------------
> > +
> 
> Please omit such extra comment lines.

Agree.  BTW, you may want to remove such lines from existing scripts:

$ grep -r '^//.*For context mode' scripts/coccinelle/
scripts/coccinelle/null/deref_null.cocci:// For context mode
scripts/coccinelle/misc/boolconv.cocci://  For context mode
scripts/coccinelle/misc/array_size.cocci://  For context mode
scripts/coccinelle/misc/struct_size.cocci://  For context mode
scripts/coccinelle/misc/newline_in_nl_msg.cocci://  For context mode
scripts/coccinelle/misc/badty.cocci://  For context mode
scripts/coccinelle/api/alloc/pool_zalloc-simple.cocci://  For context mode
scripts/coccinelle/api/alloc/zalloc-simple.cocci://  For context mode
scripts/coccinelle/api/alloc/alloc_cast.cocci://  For context mode
scripts/coccinelle/api/pm_runtime.cocci://  For context mode
scripts/coccinelle/api/resource_size.cocci://  For context mode
scripts/coccinelle/api/vma_pages.cocci://  For context mode

> > +@depends on i&&context@
> > +type T;
> > +T[] a;
> > +expression b;
> > +@@
> > +(
> > +* (a + ARRAY_SIZE(a))
> > +|
> > +* (&a[0] + ARRAY_SIZE(a))
> > +|
> > +* (&a[ARRAY_SIZE(a)])
> > +|
> > +* (&a[ARRAY_SIZE(a) - b])
> > +)
> 
> Extra space characters may be omitted directly after SmPL asterisks.

Good to know; thanks!

Although most scripts seem to be using white space (space (164) or
tab (26)) after the asterisk.  Please confirm if you prefer it removed
in new scripts.  Only 39 scripts don't have white space after it.

$ grep -rh '^\*' scripts/coccinelle/ | grep -o '^..' | sort | uniq -c
     26 *	
    164 * 
      4 *(
      1 *;
      1 *E
      1 *I
      1 *P
      2 *W
      1 *\
      1 *b
      1 *c
      3 *d
      1 *e
      4 *f
      1 *g
      2 *i
      1 *l
      4 *r
      2 *s
      1 *u
      2 *w
      5 *x
$ grep -rh '^\*' scripts/coccinelle/ | grep -o '^.\s' | sort | uniq -c | hd
00000000  20 20 20 20 20 32 36 20  2a 09 0a 20 20 20 20 31  |     26 *..    1|
00000010  36 34 20 2a 20 0a                                 |64 * .|
00000016
$ grep -rh '^\*' scripts/coccinelle/ | grep -o '^.\S' | wc -l
39


> …
> > +@r depends on (org || report)@
> 
> You may omit parentheses here.

Ok.

> …
> > +@script:python depends on report@
> > +p << r.p;
> > +@@
> > +
> > +msg="WARNING: Use ARRAY_END"
> > +coccilib.report.print_report(p[0], msg)
> 
> Would the following command variant be a bit nicer?
> 
> coccilib.report.print_report(p[0], "WARNING: opportunity for ARRAY_END()")

Sounds good.

> By the way:
> How do you think about to omit a cover letter for a single patch?

Sounds reasonable.  I like the cover letter as it holds the range-diff,
which may become a bit confusing when it's in the same email as the
patch, but maybe that's just me.

I'll send v2 in a single email.

> 
> Regards,
> Markus

Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]