cocci.inria.fr archive mirror
 help / color / mirror / Atom feed
* [Cocci] Detecting functions with dummy return value
@ 2019-11-01 14:52 Ondřej Surý
  2019-11-01 19:09 ` Julia Lawall
  0 siblings, 1 reply; 4+ messages in thread
From: Ondřej Surý @ 2019-11-01 14:52 UTC (permalink / raw)
  To: cocci

Hi,

we changed our allocator (wrapper) function to assert() instead of returning ISC_R_MEMORY.

As you can imagine there’s a lot of checks down the road that needs to be cleaned up,
so I am looking for a way to detect function that only does:

isc_result_t
foo(…) {

…
return (ISC_R_SUCCESS);
}

it could possibly be:

isc_result_t
foo(…) {
…
return (ISC_R_SUCCESS);
…
return (ISC_R_SUCCESS);
}

Looking at badcheck.cocci, it looks like I just need a block that would „match“ such functions,
but I can’t find a solid example on how to write a patch that would express:

Mark functions that just return ISC_R_SUCCESS and nothing else:

Something like this:

@match_rule@
expression E;
@@

<+... when != E != ISC_R_SUCCESS;
return (E);
...+>

@depends on match_rule@
@@

- return(...);
+ return;

But that doesn’t work for me, it matches all return()s.

Thanks,
Ondrej
_______________________________________________
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] Detecting functions with dummy return value
  2019-11-01 14:52 [Cocci] Detecting functions with dummy return value Ondřej Surý
@ 2019-11-01 19:09 ` Julia Lawall
  2019-11-01 19:22   ` Ondřej Surý
  0 siblings, 1 reply; 4+ messages in thread
From: Julia Lawall @ 2019-11-01 19:09 UTC (permalink / raw)
  To: Ondřej Surý; +Cc: cocci

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



On Fri, 1 Nov 2019, Ondřej Surý wrote:

> Hi,
>
> we changed our allocator (wrapper) function to assert() instead of returning ISC_R_MEMORY.
>
> As you can imagine there’s a lot of checks down the road that needs to be cleaned up,
> so I am looking for a way to detect function that only does:
>
> isc_result_t
> foo(…) {
>
> …
> return (ISC_R_SUCCESS);
> }
>
> it could possibly be:
>
> isc_result_t
> foo(…) {
> …
> return (ISC_R_SUCCESS);
> …
> return (ISC_R_SUCCESS);
> }

This is not possible.  ... describes control-flow paths.  Nothing happens
after a return.  Your attempt above would match functions where all
control-flow paths end with return (ISC_R_SUCCESS);, even if some of those
returns are in if branches.

For the case where ISC_R_SUCCESS is in a variable, you could try:

@@
expression E;
identifier foo;
@@

isc_result_t
foo(...) {
... when any
(
-return (ISC_R_SUCCESS);
+return;
|
E = ISC_R_SUCCESS;
...
(
-return (E);
+return;
|
return (ISC_R_SUCCESS);
)
}

julia


>
> Looking at badcheck.cocci, it looks like I just need a block that would „match“ such functions,
> but I can’t find a solid example on how to write a patch that would express:
>
> Mark functions that just return ISC_R_SUCCESS and nothing else:
>
> Something like this:
>
> @match_rule@
> expression E;
> @@
>
> <+... when != E != ISC_R_SUCCESS;
> return (E);
> ...+>
>
> @depends on match_rule@
> @@
>
> - return(...);
> + return;
>
> But that doesn’t work for me, it matches all return()s.
>
> Thanks,
> Ondrej
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[-- Attachment #2: 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] Detecting functions with dummy return value
  2019-11-01 19:09 ` Julia Lawall
@ 2019-11-01 19:22   ` Ondřej Surý
  2019-11-02  6:40     ` Julia Lawall
  0 siblings, 1 reply; 4+ messages in thread
From: Ondřej Surý @ 2019-11-01 19:22 UTC (permalink / raw)
  To: cocci

Hi Julia,

thanks for the quick response.

> On 1 Nov 2019, at 14:09, Julia Lawall <julia.lawall@lip6.fr> wrote:
> 
> This is not possible.  ... describes control-flow paths.  Nothing happens
> after a return.  Your attempt above would match functions where all
> control-flow paths end with return (ISC_R_SUCCESS);, even if some of those
> returns are in if branches.

I think that would be still fine, I mostly want the Coccinelle to select the functions
that need my attention and we also use Coccinelle in our CI pipelines, so we don’t
reintroduce the code we already cleaned up.

So, if I could just mark the functions that *only* return ISC_R_SUCCESS anywhere,
it would be enough for my use case.  I would then go through the results by hand
and write a targeted spatch for every selected function.  That’s fine, even though
Coccinelle feels like a magic wand, I don’t expect it to be one.

The spatch you sent ends up with:

minus: parse error:
  File "cocci/dummy.spatch", line 7, column 0, charpos = 50
  around = 'foo',
  whole content = foo(...) {

BTW Coccinelle is such a great tool, it has helped with BIND 9 refactoring so much,
so thank you for working on that!

Thanks,
Ondrej
_______________________________________________
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] Detecting functions with dummy return value
  2019-11-01 19:22   ` Ondřej Surý
@ 2019-11-02  6:40     ` Julia Lawall
  0 siblings, 0 replies; 4+ messages in thread
From: Julia Lawall @ 2019-11-02  6:40 UTC (permalink / raw)
  To: Ondřej Surý; +Cc: cocci

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



On Fri, 1 Nov 2019, Ondřej Surý wrote:

> Hi Julia,
>
> thanks for the quick response.
>
> > On 1 Nov 2019, at 14:09, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> > This is not possible.  ... describes control-flow paths.  Nothing happens
> > after a return.  Your attempt above would match functions where all
> > control-flow paths end with return (ISC_R_SUCCESS);, even if some of those
> > returns are in if branches.
>
> I think that would be still fine, I mostly want the Coccinelle to select the functions
> that need my attention and we also use Coccinelle in our CI pipelines, so we don’t
> reintroduce the code we already cleaned up.
>
> So, if I could just mark the functions that *only* return ISC_R_SUCCESS anywhere,
> it would be enough for my use case.  I would then go through the results by hand
> and write a targeted spatch for every selected function.  That’s fine, even though
> Coccinelle feels like a magic wand, I don’t expect it to be one.
>
> The spatch you sent ends up with:
>
> minus: parse error:
>   File "cocci/dummy.spatch", line 7, column 0, charpos = 50
>   around = 'foo',
>   whole content = foo(...) {

Sorry, you also need:

typedef isc_result_t;

in the metavariable list.

>
> BTW Coccinelle is such a great tool, it has helped with BIND 9
> refactoring so much, so thank you for working on that!

Nice to hear :) Thanks.

julia

[-- Attachment #2: 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

end of thread, other threads:[~2019-11-02  6:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-01 14:52 [Cocci] Detecting functions with dummy return value Ondřej Surý
2019-11-01 19:09 ` Julia Lawall
2019-11-01 19:22   ` Ondřej Surý
2019-11-02  6:40     ` Julia Lawall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).