Re: [Cocci] Make rule depend on comment

["Christoph Böhmwalder" <christoph.boehmwalder@linbit.com>]

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.
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