On Thu, 2 Sep 2010, Paul E. McKenney wrote:
> On Thu, Sep 02, 2010 at 07:25:02AM +0200, Julia Lawall wrote:
> > > > This time there is a call to foo after the call to bar and before the
> > > > call
> > > > to xxx, and this call to foo is that same as the one that was matched
> > > > before the call to bar.
> > >
> > > OK, because we would have to cycle around the loop to match, and the
> > > "when" clause would disqualify the only reasonable match.
> >
> > Yes.
> >
> > > Thank you again!
> > >
> > > And yet another question...
> > >
> > > spatch does not seem to like the following:
> > >
> > > when != if (p) atomic_inc(&<+...p->f1...+>)
> >
> > The branch of an if is a statement. A function call that is a statement
> > has a ; after it. I see that the parse error is quite unhelpful, due to
> > the way the yacc-like parsing works.
>
> Ah, got it!
>
> > But here you assume that the if (p) will only have the atomic_inc.
> > Perhaps the following would be better:
> >
> > when != if (p) { ... atomic_inc(&<+...p->f1...+>); ... }
> >
> > Note that all of the when code has to stay on the same line. An
> > isomorphism will allow this to match a case where there are no {}.
>
> This does work nicely, thank you! The isomorphism works in both
> directions?
No. You can find the definition in standard.iso. But in general,
isomorphisms only remove things. Adding things would probably result in
too many permutations.
> > > The problem I am trying to solve is that it is OK to leak a pointer
> > > out of an RCU read-side critical section if you have done something
> > > to keep the underlying data structure from going away. One way of
> > > doing this is to increment a reference count in the pointed-to structure,
> > > for example:
> > >
> > > rcu_read_lock();
> > > p = rcu_dereference(gp);
> > > if (p && something_bad_happened(p))
> > > p = NULL
> > > if (p)
> > > atomic_inc(&p->refcount);
> > > rcu_read_unlock();
> > >
> > > if (p)
> > > do_something_with(p->a);
> > >
> > > My current set of rules will flag the last line as a pointer leak,
> > > presumably because there is a code path that skips the atomic_inc().
> > > My first reaction was to just put the "if" statement in a when clause
> > > as above, but that gives a parse error.
> > >
> > > I tried a number of variations, including a separate rule matching the
> > > "if" (which worked) and recording the "if"s position (which did not).
> > > For your amusement, one attempt follows.
> >
> > I don't think when != @guardedref.q works at all, or if it does, then it
> > is being interpreted as something other than what you intended. When you
> > inherit a metavariable, the name of the rule from which it is inherited
> > only appears in the metavariable declaration. So guardedref.q is a
> > structure field reference. And a position variable has to be attached to
> > something. In this case, it should be an identifier, because that is what
> > the position was attached to in guardedref. It can be any identifier at
> > all, not necessarily an inherited one, because the position is enough to
> > make it unique.
>
> OK, so the rule inheriting the metavariable must not have another
> metavariable with that same name. Fair enough!
Yes, this is the case.
julia
_______________________________________________
Cocci mailing list
[email protected]
http://lists.diku.dk/mailman/listinfo/cocci
(Web access from inside DIKUs LAN only)
