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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4DF7D197.5070205@cuw.edu>
Date:	Tue, 14 Jun 2011 16:24:39 -0500
From:	Greg Dietsche <gregory.dietsche@....edu>
To:	Julia Lawall <julia@...u.dk>
CC:	Gilles.Muller@...6.fr, npalix.work@...il.com, cocci@...u.dk,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] coccinelle: if (ret) return ret; return ret; semantic
 patch

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
>    


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