[coreboot] [PATCH] flashrom: Remove superfluous volatile

[Carl-Daniel Hailfinger]

On 06.03.2009 00:59, Joseph Smith wrote:
> On Fri, 06 Mar 2009 00:23:45 +0100, Carl-Daniel Hailfinger
> <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>   
>> During the conversion of flash chip accesses to helper functions, I
>> spotted assignments to volatile variables which were neither placed
>> inside the mmapped ROM area nor were they counters.
>> Due to the use of accessor functions, volatile usage can be reduced
>> significantly because the accessor functions take care of actually
>> performing the reads/writes correctly.
>>
>> The following semantic patch spotted them:
>> r exists@
>> expression b;
>> typedef uint8_t;
>> volatile uint8_t a;
>> position p1;
>> @@
>>  a at p1 = readb(b);
>>
>> @script:python@
>> p1 << r.p1;
>> a << r.a;
>> b << r.b;
>> @@
>> print "* file: %s line %s has assignment to unnecessarily volatile
>> variable: %s = readb(%s);" % (p1[0].file, p1[0].line, a, b)
>>
>> Result was:
>> HANDLING: sst28sf040.c
>> * file: sst28sf040.c line 44 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 43 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 42 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 41 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 40 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 39 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 38 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 58 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 57 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 56 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 55 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 54 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 53 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>> * file: sst28sf040.c line 52 has assignment to unnecessarily volatile
>> variable: tmp = readb(TODO: Binary);
>>
>> From that result, the fix was obvious:
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>>     
> Acked-by: Joseph Smith <joe at settoplinux.org>
>
> Good find. Looks like your having fun with the semantic patching :-)
>   

Thanks.

By the way, the following semantic patch uses the builtin match printing
functionality by prepending a "*" to the line with the pattern:
@@
expression b;
typedef uint8_t;
volatile uint8_t a;
@@
* a = readb(b);

Result is:
HANDLING: sst28sf040.c
diff =
--- sst28sf040.c        2009-03-06 01:04:49.000000000 +0100
@@ -35,13 +35,6 @@ static __inline__ void protect_28sf040(v
        /* ask compiler not to optimize this */
        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);
 }
 
 static __inline__ void unprotect_28sf040(volatile uint8_t *bios)
@@ -49,13 +42,6 @@ static __inline__ void unprotect_28sf040
        /* ask compiler not to optimize this */
        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 + 0x041A);
 }
 
 static __inline__ int erase_sector_28sf040(volatile uint8_t *bios,

It's arguably a bit easier to read if you get used to the leading "-"
for matching lines.

This patch was enabled by Coccinelle: http://www.emn.fr/x-info/coccinelle/

Committed in r3973.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/