linux-kernel - Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH] checkpatch: Add test for comma use that should be semicolon)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.22.394.2008201021400.2524@hadrien>
Date:   Thu, 20 Aug 2020 10:33:33 +0200 (CEST)
From:   Julia Lawall <julia.lawall@...ia.fr>
To:     Joe Perches <joe@...ches.com>
cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Julia Lawall <julia.lawall@...6.fr>,
        cocci <cocci@...teme.lip6.fr>,
        Giuseppe Scrivano <gscrivan@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Andy Whitcroft <apw@...dowen.org>
Subject: Re: [Cocci] coccinelle: Convert comma to semicolons (was Re: [PATCH]
 checkpatch: Add test for comma use that should be semicolon)



On Wed, 19 Aug 2020, Joe Perches wrote:

> On Wed, 2020-08-19 at 14:22 -0700, Joe Perches wrote:
> > There are commas used as statement terminations that should typically
> > have used semicolons instead.  Only direct assignments or use of a single
> > function or value on a single line are detected by this test.
> >
> > e.g.:
> > 	foo = bar(),		/* typical use is semicolon not comma */
> > 	bar = baz();
> >
> > Add an imperfect test to detect these comma uses.
> >
> > No false positives were found in testing, but many types of false negatives
> > are possible.
> >
> > e.g.:
> > 	foo = bar() + 1,	/* comma use, but not direct assignment */
> > 	bar = baz();
>
> Hi.
>
> I recently added a test for this condition to linux's checkpatch.
>
> A similar coccinelle script might be:
>
> $ cat comma.cocci
> @@
> expression e1;
> expression e2;
> @@
>
> 	e1
> -	,
> +	;
> 	e2;
> $
>
> This works reasonably well but it has several false positives
> for declarations like:
>
> $ spatch --sp-file comma.cocci mm/huge_memory.c
> diff -u -p a/huge_memory.c b/huge_memory.c
> --- a/huge_memory.c
> +++ b/huge_memory.c
> @@ -2778,7 +2778,7 @@ static unsigned long deferred_split_scan
>  	struct pglist_data *pgdata = NODE_DATA(sc->nid);
>  	struct deferred_split *ds_queue = &pgdata->deferred_split_queue;
>  	unsigned long flags;
> -	LIST_HEAD(list), *pos, *next;
> +	LIST_HEAD(list), *pos; *next;
>  	struct page *page;
>  	int split = 0;
> $
>
> Any script improvement suggestions?

I have a bunch of variations of this that are more complicated than I
would have expected.  One shorter variant that I have is:

@@
expression e1,e2;
statement S;
@@

 S
 e1
-,
+;
  (<+... e2 ...+>);

This will miss cases where the first statement is the comma thing.  But I
think it is possible to improve this now.  I will check.

thanks,
julia