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

[Julia Lawall]

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