[Cocci] "TODO: FunCall" in python match printing

[Carl-Daniel Hailfinger]

On 06.03.2009 00:59, Yoann Padioleau wrote:
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> writes:
>
>   
>> Hi,
>>
>> I decided to use spatch for diagnostics on the refactored flashrom code.
>> flashrom deals with direct memory accesses, so the code is literally
>> sprinkled with volatile qualifiers for variables. Some of them don't
>> make sense, especially assignments to variables which are not placed in
>> mmapped flash ROM space.
>>
>> The following semantic patch does almost what I want:
>>
>> @r exists@
>> expression b;
>> typedef uint8_t;
>> volatile uint8_t a;
>> position p1;
>> @@
>>  a at p1 = b;
>>
>> @script:python@
>> p1 << r.p1;
>> a << r.a;
>> b << r.b;
>> @@
>> print "* file: %s line %s has assignment to unnecessarily volatile
>> variable: %s = %s;" % (p1[0].file, p1[0].line, a, b)
>>     
>
> Note that you can also use the 'sgrep' mode to find code. Just put a '*'
> in front of the line you want to display.
>   

Nice. That saves me a lot of code.


> @@
> expression b; 
> typedef uint8_t;
> volatile uint8_t a;
> @@
>
> * a = b;
>
>
> You can also abuse the - and + of spatch and not modify in-place
> the file and do
>
> @@
> expression b; 
> typedef uint8_t;
> volatile uint8_t a;
> @@
>
> - a = b;
> + HERE_IS_CODE_YOU_SHOULD_LOOK();
>
> but it's less elegant.
>   

I actually had something similar at first, but I wanted a "correct"
solution.
@@
expression b;
typedef uint8_t;
volatile uint8_t a;
@@
- a = readb(b);
+ a = SUPERFLUOUSVOLATILE + readb(b);


> Of course python is also perfectly fine, but with spatch/sgrep
> you can also directly see the context of the code as produced
> by diff (but python may make it easier to go to the place
> if you are under emacs compilation mode).
>   

Good to know.


>> Original code:
>>
>> volatile uint8_t tmp;
>> tmp = readb(bios + 0x1823);
>> tmp = readb(bios + 0x1820);
>> tmp = readb(bios + 0x1822);
>> tmp = readb(bios + 0x0418);
>> tmp = readb(bios + 0x041B);
>> tmp = readb(bios + 0x0419);
>> tmp = readb(bios + 0x040A);
>>
>> Generated messages:
>> * file: sst28sf040.c line 44 has assignment to unnecessarily volatile
>> variable: tmp = TODO: FunCall;
>> * file: sst28sf040.c line 43 has assignment to unnecessarily volatile
>> variable: tmp = TODO: FunCall;
>> * file: sst28sf040.c line 42 has assignment to unnecessarily volatile
>> variable: tmp = TODO: FunCall;
>> * file: sst28sf040.c line 41 has assignment to unnecessarily volatile
>> variable: tmp = TODO: FunCall;
>> * file: sst28sf040.c line 40 has assignment to unnecessarily volatile
>> variable: tmp = TODO: FunCall;
>> * file: sst28sf040.c line 39 has assignment to unnecessarily volatile
>> variable: tmp = TODO: FunCall;
>> * file: sst28sf040.c line 38 has assignment to unnecessarily volatile
>> variable: tmp = TODO: FunCall;
>>
>> I'd prefer to print the whole line, but as long as the TODO remains, 
>>     
>
>
> Hey Julia, it's not even my TODO :) (private joke).
>
>   
>> the
>> current output will have to suffice. Actually, the output is good enough
>> for my purposes.
>>
>> Just noticed: For some files, the above patch gives a warning:
>>      (ONCE) Missing type information. Certainly a pb in annotate_typer.ml
>>     
>
> That's because of the way we parse the code. We try to parse as-is
> the code (without calling cpp), which means that sometimes
> we miss some typing information. We try to infer many things but
> sometimes it does not work.
>   

OK. Do you want these files as test cases?


>> For some other files, the warning is different:
>> (ONCE) ast_to_flow: filter a directive
>>     
>
> Ok, this one is mine ... it tells you that the engine
> will not represent some cpp directive like #ifdef or #define instructions
> in the internal structure of coccinelle. You should not be concerned
> by that, it's mainly for the developer of coccinelle.
>   

OK, thanks.

I just reran spatch without -no_includes -noif0_passing and got the
following warning instead:
(ONCE) Type annotater:not handling GetRefLabel

If you want testcases, just ask. The code is GPLv2 and freely downloadable.


> (Julia, maybe we can do pr2_developer/pr2_user ? or maybe just put
> those pr2 behind a flag).
>
>
>   
>> I'll follow up with links to the mailing list post and the commit later.
>>     
>
> Cool.
>   

Post with first variant of semantic patch (python printing):
http://www.coreboot.org/pipermail/coreboot/2009-March/045351.html
Second variant (sgrep functionality):
http://www.coreboot.org/pipermail/coreboot/2009-March/045355.html
Commit:
http://www.coreboot.org/pipermail/coreboot/2009-March/045354.html


Regards,
Carl-Daniel