Coccinelle Archive on lore.kernel.org
 help / color / Atom feed
* [Cocci] Removing the last return statement from a void function
@ 2021-03-18 18:26 Thomas Adam
  2021-03-18 19:24 ` Julia Lawall
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Adam @ 2021-03-18 18:26 UTC (permalink / raw)
  To: Coccinelle

Hello all,

I've another Coccinelle question I'm hoping you can help me with.  The
codebase I'm working on is old, and has some interesting styles which
by themselves probably don't cause any problems, but newer C compilers
are now starting to flag them.

In particular, there seems to be a pattern in this code base of using
explicit `return;` statements at the end of void functions.  Here's an
example:

static void broadcast_mini_icon(FvwmWindow *fw)
{
    if (!FMiniIconsSupported)
    {
        return;
    }
    if (fw->mini_pixmap_file && fw->mini_icon)
    {
        BroadcastFvwmPicture( M_MINI_ICON, FW_W(fw),
            FW_W_FRAME(fw), (unsigned long)fw,
            fw->mini_icon, fw->mini_pixmap_file);
    }
    return;
}

Here you can see the last return statement is not necessary.

I'm trying to make coccinelle recognise this and remove such cases.
Here's what I've tried:

@@
identifier f;
@@

void f(...) {
  <...
- return;
...>

}

... which sort of works, but proceeds to remove *all* `return;`
statements from void functions, rather than the last occurance in the
function.

Am I on the right track with this approach, or do I need to do
something more creative?

Thanks once more for your help.

Thomas
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Cocci] Removing the last return statement from a void function
  2021-03-18 18:26 [Cocci] Removing the last return statement from a void function Thomas Adam
@ 2021-03-18 19:24 ` Julia Lawall
  2021-03-19 20:40   ` Thomas Adam
  0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2021-03-18 19:24 UTC (permalink / raw)
  To: Thomas Adam; +Cc: Coccinelle



On Thu, 18 Mar 2021, Thomas Adam wrote:

> Hello all,
>
> I've another Coccinelle question I'm hoping you can help me with.  The
> codebase I'm working on is old, and has some interesting styles which
> by themselves probably don't cause any problems, but newer C compilers
> are now starting to flag them.
>
> In particular, there seems to be a pattern in this code base of using
> explicit `return;` statements at the end of void functions.  Here's an
> example:
>
> static void broadcast_mini_icon(FvwmWindow *fw)
> {
>     if (!FMiniIconsSupported)
>     {
>         return;
>     }
>     if (fw->mini_pixmap_file && fw->mini_icon)
>     {
>         BroadcastFvwmPicture( M_MINI_ICON, FW_W(fw),
>             FW_W_FRAME(fw), (unsigned long)fw,
>             fw->mini_icon, fw->mini_pixmap_file);
>     }
>     return;
> }
>
> Here you can see the last return statement is not necessary.
>
> I'm trying to make coccinelle recognise this and remove such cases.
> Here's what I've tried:
>
> @@
> identifier f;
> @@
>
> void f(...) {
>   <...
> - return;
> ...>
>
> }
>
> ... which sort of works, but proceeds to remove *all* `return;`
> statements from void functions, rather than the last occurance in the
> function.
>
> Am I on the right track with this approach, or do I need to do
> something more creative?

The ... in Coccinelle is based on control flow, so it is a bit hard to
find the return at the bottom of the function.  Actually, from
Coccinelle's point of view, all returns are at the bottom of the function,
because one leaves the function after a return.

You can try the following:

@r@
position p;
identifier f;
}

f(...) {
<...
{ .. return@p; }
...>
}

@@
position p != r.p;
@@

- return@p;

Basically the first rule collects the position of all returns that are
inside a { }, and then the second rule removes the others.

However there is an isomorphism that makes a pattern with { ... S } match
just S, for any S, which you don't want.  So you can make an empty file
called empty.iso, and then run the rule with the command-line argument
--iso-file empty.iso

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Cocci] Removing the last return statement from a void function
  2021-03-18 19:24 ` Julia Lawall
@ 2021-03-19 20:40   ` Thomas Adam
  2021-03-19 21:10     ` Julia Lawall
  0 siblings, 1 reply; 4+ messages in thread
From: Thomas Adam @ 2021-03-19 20:40 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Coccinelle


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

On Thu, 18 Mar 2021 at 19:24, Julia Lawall <julia.lawall@inria.fr> wrote:

> The ... in Coccinelle is based on control flow, so it is a bit hard to
> find the return at the bottom of the function.  Actually, from
> Coccinelle's point of view, all returns are at the bottom of the function,
> because one leaves the function after a return.

Interesting, that helps me understand a little more about Coccinelle.  Thanks.

> You can try the following:
>
> @r@
> position p;
> identifier f;
> }
>
> f(...) {
> <...
> { .. return@p; }
> ...>
> }
>
> @@
> position p != r.p;
> @@
>
> - return@p;

So I tried this:

@r@
position p;
identifier f;
@@

f(...) {
<...
   { ... return@p; }
...>
}

@@
position p != r.p;
@@

- return@p;

Which I ran as:

spatch --in-place --debug --iso-file contrib/coccinelle/empty.iso \
    --sp-file ./contrib/coccinelle/remove-void-return.cocci --dir fvwm

With "--dir fvwm", I found that my CPU was being chewed at 100%, which I left
running overnight.  Some 8 hours later, spatch was still running.  Presumably,
Coccinelle is having an interesting time coordinating the positions?

Instead, I decided to loop over the .c files which "--dir fvwm" would have
done.  What I found was that for some files, spatch took a few seconds, and
produced no output, yet for some, spatch was still running without any result
known (so I killed it).

Indeed, I'm attaching a debug run of spatch to this email (cocci-debug) for
one file that definitely has functions where I would expect Coccinelle to have
matched a "return;" statement to be removed, but this wasn't the case.

Would you be able to suggest what I might have done wrong, or if there's any
additional debugging I can provide?

Thanks,
Thomas

[-- Attachment #2: cocci-debug --]
[-- Type: application/octet-stream, Size: 18953 bytes --]

[-- Attachment #3: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Cocci] Removing the last return statement from a void function
  2021-03-19 20:40   ` Thomas Adam
@ 2021-03-19 21:10     ` Julia Lawall
  0 siblings, 0 replies; 4+ messages in thread
From: Julia Lawall @ 2021-03-19 21:10 UTC (permalink / raw)
  To: Thomas Adam; +Cc: Coccinelle



On Fri, 19 Mar 2021, Thomas Adam wrote:

> On Thu, 18 Mar 2021 at 19:24, Julia Lawall <julia.lawall@inria.fr> wrote:
>
> > The ... in Coccinelle is based on control flow, so it is a bit hard to
> > find the return at the bottom of the function.  Actually, from
> > Coccinelle's point of view, all returns are at the bottom of the function,
> > because one leaves the function after a return.
>
> Interesting, that helps me understand a little more about Coccinelle.  Thanks.
>
> > You can try the following:
> >
> > @r@
> > position p;
> > identifier f;
> > }
> >
> > f(...) {
> > <...
> > { .. return@p; }
> > ...>
> > }
> >
> > @@
> > position p != r.p;
> > @@
> >
> > - return@p;
>
> So I tried this:
>
> @r@
> position p;
> identifier f;
> @@
>
> f(...) {
> <...
>    { ... return@p; }
> ...>
> }
>
> @@
> position p != r.p;
> @@
>
> - return@p;
>
> Which I ran as:
>
> spatch --in-place --debug --iso-file contrib/coccinelle/empty.iso \
>     --sp-file ./contrib/coccinelle/remove-void-return.cocci --dir fvwm
>
> With "--dir fvwm", I found that my CPU was being chewed at 100%, which I left
> running overnight.  Some 8 hours later, spatch was still running.  Presumably,
> Coccinelle is having an interesting time coordinating the positions?
>
> Instead, I decided to loop over the .c files which "--dir fvwm" would have
> done.  What I found was that for some files, spatch took a few seconds, and
> produced no output, yet for some, spatch was still running without any result
> known (so I killed it).
>
> Indeed, I'm attaching a debug run of spatch to this email (cocci-debug) for
> one file that definitely has functions where I would expect Coccinelle to have
> matched a "return;" statement to be removed, but this wasn't the case.
>
> Would you be able to suggest what I might have done wrong, or if there's any
> additional debugging I can provide?

If Coccinelle gets stuck on one part of a file, it won't move on to the
next part.  So the fact that the something should have been done in the
file is not really helpful.

You can use the argument --show-trying to see what function it is working
on at the moment.

You can use --timeout N to limit the treatment of a given file to N
seconds.

It may help to replace @r@ by @r exists@.

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 18:26 [Cocci] Removing the last return statement from a void function Thomas Adam
2021-03-18 19:24 ` Julia Lawall
2021-03-19 20:40   ` Thomas Adam
2021-03-19 21:10     ` Julia Lawall

Coccinelle Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/cocci/0 cocci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 cocci cocci/ https://lore.kernel.org/cocci \
		cocci@systeme.lip6.fr
	public-inbox-index cocci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/fr.lip6.systeme.cocci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git