> > @r exists@
> > identifier f;
> > position p;
> > @@
> >
> > f(...) {
> >   ...
> >   RETURN@p;
> > }
> >
> > @@
> > position r.p;
> > @@
> >
> > - RETURN@p;
> >
> > @@
> > @@
> >
> > - RETURN;
> > + return;
>
> Cute.  I'm not sure why you need the two separate rules with matching
> positions; consolidating the two middle rules into one seems to work as
> well.  I did have to add a rule for when the return immediately followed
> a label, to make sure there was a statement to target.

I was worried that putting the rules together would cause an already
tagged token error, when there are several paths to the return, via gotos.

> However, this seems to have two issues.  First, to some extent I
> *wanted* the control-flow matching, to handle cases like this without
> having to do so explicitly:
>
> void f(void)
> {
>       if (...) {
>               foo();
> -             return;
>       }
> }

This return is clearly not just before the ending brace...
You could have a separate rule for this case, but then there is still the
case where there are two nested ifs, etc.

> Second, this seems to lead to many formatting issues:
>
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -895,7 +895,7 @@ static void print_other_cpu_stall(struct
>
>       rcu_print_detail_task_stall(rsp);
>
> -     force_quiescent_state(rsp);  /* Kick them all. */
> +     force_quiescent_state(rsp);
>  }
>
>  static void print_cpu_stall(struct rcu_state *rsp)
>
> No idea why coccinelle touched this comment.  Do functions have an
> implicit "return;" at the end that got patched here?

Yes, functions have an implicit return.  I was hoping that it wouldn't get
matched if the return was not written inside braces, but that doesn't seem
to have been successful...

> --- a/lib/xz/xz_crc32.c
> +++ b/lib/xz/xz_crc32.c
> @@ -43,7 +43,7 @@ XZ_EXTERN void xz_crc32_init(void)
>               xz_crc32_table[i] = r;
>       }
>
> -     return;
> +     RETURN;
>  }
>
>  XZ_EXTERN uint32_t xz_crc32(const uint8_t *buf, size_t size, uint32_t crc)
>
> No idea what happened here to prevent the RETURN; from getting
> substituted back.

Maybe a parsing problem?  I will look into it.

> Also, I noticed some cases where the return; in question immediately
> followed a label, so removing it would give that label no statement to
> jump to.  I tried handling that explicitly:
>
> @r1 depends on patch exists@
> identifier f, l;
> statement S;
> position p;
> @@
> void f(...)
> {
> ...
> (
> l:
> - RETURN;@p
>   S
> |
> l:
> - RETURN@p
>   ;
> |
> - RETURN;@p
> )
> }
>
> However, while this did produce the desired result when the return
> immeidately followed the label, it produced odd results in other cases:
>
> --- a/crypto/seqiv.c
> +++ b/crypto/seqiv.c
> @@ -45,6 +45,7 @@ static void seqiv_complete2(struct skcip
>
>  out:
>       kfree(subreq->info);
> +     return;
>  }
>
>  static void seqiv_complete(struct crypto_async_request *base, int err)
>
> Many instances like this where coccinelle added "return;" where none
> existed before.

I don't think this has to do with the label issue, but with the implicit
return issue.

julia
_______________________________________________
Cocci mailing list
[email protected]
https://systeme.lip6.fr/mailman/listinfo/cocci

Reply via email to