Re: [PATCH v2] coccinelle: if (ret) return ret; return ret; semanticpatch

[Greg Dietsche]

On 06/13/2011 01:38 PM, Julia Lawall wrote:
> How about:
>
> @@
> identifier f;
> expression ret;
> identifier x;
> @@
>
> (
> - if (likely(x)) return ret;
> |
> - if (\(IS_ERR\|IS_ZERO\|is_ordinal_table\)(x)) return ret;
> |
>    if (<+...f(...)...+>) return ret;
> |
> - if (...) return ret;
> )
> return ret;
>
>    
just curious... i see you usually just write "return ret;" here when 
posting. I've assumed that's because it will 1) work and 2) is close enough.

You'll notice I've been doing:
-return ret;
+return ret;
because it seems to help coccinelle realize that it can get rid of extra 
line feeds - does this make sense - or should i just be doing a "return 
ret"?
> I have put the likely case separate from the other function calls to
> benefit from the isomorphism.  I have restricted the argument to these
> functions to be an identifier so that it won't have any side effects.  It
> doesn't have to be the same as ret though.  The third line keeps all other
> ifs that contain function calls.  The fourth line gets rid of everything
> else.
>
> You could see if this finds all of the cases of your proposed rule and if
> it at least doesn't find anything else that you don't want it to find.
>
>    
I'll try it out this afternoon/evening hopefully.
> julia
>
>    

There are two other issues with the patch that I've noticed. I'll be 
teaching myself more on coccinelle to figure these out. Unless someone 
else wants to jump in :) So far I've read or skimmed a number of paper's 
that have been written on Coccinelle... I find it all very interesting :)

1) sometimes you see this type of code - which i've chosen to ignore for 
now:
if ((ret=XXXXX) < 0)
    return ret;
return ret;

which could just be simplified to:
return XXXXX;

for an example see the function load_firmware in 
sound/pci/echoaudio/echoaudio_dsp.c

2) after the semantic patch has removed an "if (...)return ret;" Quite 
often, but not always, we end up with this:
ret=...;
return ret;

which of course could just become
return ...;


So as you can see the problems are quite similar, but a little different.

Greg

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/