Re: [Cocci] Make rule depend on comment

[Julia Lawall <Julia.Lawall@lip6.fr>]

Le 05.08.2019 10:36, Christoph Böhmwalder a écrit :
> Thanks for your reply!
> 
>> But in your case, would it suffice just to put an if in the python 
>> code?
>> Or is it essential that the first rule actually not match?
>> 
>> julia
> I'm pretty sure the best way to go about this is to have the script 
> rule
> actually not match.

OK, this can be done by associating script code with the metavariable 
and having the script code check that the set of comments is non-empty.  
I'll try to add this within the next day.

julia

> Allow me to outline my real world use-case for this:
> 
> Basically, we're building a kernel compatibility layer based on 
> coccinelle for
> an out-of-tree module.
> This particular semantic patch circles around the k[un]map_atomic() 
> functions.
> Specifically, somewhere in the kernel's history, the way to use these 
> functions
> went from this:
> 
>     void *addr = kmap_atomic(page, KM_USER0);
> 
> to this:
> 
>     void *addr = kmap_atomic(page);
> 
> As we're looking to eventually upstream our module, we obviously cannot 
> have the
> old KM_* constants lying around in our code, so we somehow need to 
> restore the
> semantic information that was lost by removing this parameter.
> The way we decided to go about this is to add a "tag" to each function
> containing
> a k[un]map_atomic call; imagine the following:
> 
>     int foo()
>     /* kmap compat: KM_USER0 */
>     {
>         void *addr = kmap_atomic(page);
>         // ...
>         kunmap_atomic(addr);
>     }
> 
> It would now be coccinelles job to make sure that each k[un]map_atomic 
> call is
> augmented by this legacy parameter (KM_USER0 in this case). Here's the 
> semantic
> patch I came up with to accomplish this:
> 
> 
>     @ find_kmap_tagged_function @
>     comments tag;
>     identifier fn;
>     @@
>     fn(...)@tag {
>     ...
>     }
> 
>     @ script:python parse_kmap_tag @
>     tag << find_kmap_tagged_function.tag;
>     km;
>     @@
>     import re
>     match = re.search(r'^\/\*\skmap compat: (.*)\s\*\/$', tag[0].after)
>     if match:
>         coccinelle.km = match.group(1)
> 
>     @@
>     expression page, addr;
>     identifier find_kmap_tagged_function.fn;
>     identifier parse_kmap_tag.km;
>     @@
>     fn(...) {
>     <...
>     (
>     -kmap_atomic(page)
>     +kmap_atomic(page, km)
>     |
>     -kunmap_atomic(addr)
>     +kunmap_atomic(addr, km)
>     )
>     ...>
>     }
> 
> 
> The first rule finds the comment between the argument list and the 
> opening curly
> brace, the second rule parses out the KM_* constant using a regex, and 
> the third
> rule replaces all calls to k[un]map_atomic -- inserting the correct 
> parameter.
> 
> This works just fine, except for the case where no "tag" was found. 
> Then spatch
> produces output like:
> 
> 
>      int foo()
>      {
>             int page = 1234;
>     -       void *addr = kmap_atomic(page);
>     +       void *addr = kmap_atomic(page,
>     +                                initial value: consider using
> coccinelle.varname);
>             // ...
>     -       kunmap_atomic(addr);
>     +       kunmap_atomic(addr, initial value: consider using
> coccinelle.varname);
> 
> 
> Which seems to me like it would confuse someone who doesn't know 
> exactly what
> patching is done behind the scenes.
> 
> Ideally, I would like for coccinelle to not touch the function at all
> if it doesn't
> have a "tag". I'm not sure if this is objectively the best solution,
> but it seems
> the most logical to me.
> 
> I'd be glad to hear any input you might have regarding this.
> 
> Again, thanks for all the help!
> 
> --
> Christoph Böhmwalder
> LINBIT | Keeping the Digital World Running
> DRBD HA —  Disaster Recovery — Software defined Storage
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci