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

[Greg Dietsche <gregory.dietsche@cuw.edu>]

On 06/14/2011 12:50 AM, Julia Lawall wrote:
> On Mon, 13 Jun 2011, Greg Dietsche wrote:
>    
>> 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 wondered why you were doing that :)
>
> Is the problem that the removed if is being replaced by a blank line?  If
> so, that is not intentional.  I will look into it.  If it doesn't happen
> always, an example where it does happen could be helpful.
>
>    

Some times it gets it right, so it's not always wrong. For example:

diff -u -p a/arch/m68k/math-emu/fp_log.c b/arch/m68k/math-emu/fp_log.c
--- a/arch/m68k/math-emu/fp_log.c 2011-06-13 14:06:37.943954469 -0500
+++ b/arch/m68k/math-emu/fp_log.c 2011-06-14 16:07:22.394954040 -0500
@@ -105,9 +105,6 @@ fp_fetoxm1(struct fp_ext *dest, struct f

         fp_monadic_check(dest, src);

-       if (IS_ZERO(dest))
-               return dest;
-
         return dest;
  }



Here's an example where it got it "wrong" - notice how the blank line is 
missing the - :

diff -u -p a/arch/frv/mm/pgalloc.c b/arch/frv/mm/pgalloc.c
--- a/arch/frv/mm/pgalloc.c 2011-06-13 14:06:37.855954391 -0500
+++ b/arch/frv/mm/pgalloc.c 2011-06-14 16:07:16.714954008 -0500
@@ -136,8 +136,6 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
         pgd_t *pgd;

         pgd = quicklist_alloc(0, GFP_KERNEL, pgd_ctor);
-       if (!pgd)
-               return pgd;

         return pgd;
  }


but when I do
-return ret;
+return ret;

then both of the above examples are "correct"


Greg


> The disadvantage of removing something and then adding it back is that
> then Coccinelle takes over the pretty printing of that thing.  Since ret
> is usually a variable, it doesn't matter, and since Coccinelle tries to
> follow Linux spacing conventions it might not matter either.  But eg
> f(a,b,c,d) would become f(a, b, c, d).
>
> julia
>