Re: [coreboot] [PATCH] flashrom: Remove superfluous volatile

[Joseph Smith]

On Fri, 06 Mar 2009 00:23:45 +0100, Carl-Daniel Hailfinger
 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@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

Acked-by: Joseph Smith

Good find. Looks like your having fun with the semantic patching :-)
> Index: flashrom-unneeded_volatile/sst28sf040.c
>
> --- flashrom-unneeded_volatile/sst28sf040.c	(Revision 3971)
> +++ flashrom-unneeded_volatile/sst28sf040.c	(Arbeitskopie)
> @@ -32,8 +32,7 @@
>
> static __inline__ void protect_28sf040(volatile uint8_t *bios)
>  {
> /* ask compiler not to optimize this */
> volatile uint8_t tmp;
>
> uint8_t tmp;
>
> tmp = readb(bios + 0x1823);
> tmp = readb(bios + 0x1820);
>
> @@ -46,8 +45,7 @@
>
> static __inline__ void unprotect_28sf040(volatile uint8_t *bios)
>  {
> /* ask compiler not to optimize this */
> volatile uint8_t tmp;
>
> uint8_t tmp;
>
> tmp = readb(bios + 0x1823);
> tmp = readb(bios + 0x1820);