This is not just for fun. We hit a real bug on BSD with the outl macros. The macro variable tmp collided with the tmp from outer scope.
second revision, now also taking care of inb/inw/inl. While that shadowing did not introduce bugs (yet), of course it breaks the build on BSD when -Wshadow is enabled.
Signed-off-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de --- Makefile | 2 +- cbtable.c | 8 ++++---- cli_classic.c | 1 - flash.h | 1 - hwaccess.h | 12 ++++++------ 5 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/Makefile b/Makefile index 423e315..501f65a 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ INSTALL = install DIFF = diff PREFIX ?= /usr/local MANDIR ?= $(PREFIX)/share/man -CFLAGS ?= -Os -Wall -Werror +CFLAGS ?= -Os -Wall -Werror -Wshadow EXPORTDIR ?= .
OS_ARCH = $(shell uname) diff --git a/cbtable.c b/cbtable.c index 2bc1bfa..2611a62 100644 --- a/cbtable.c +++ b/cbtable.c @@ -50,7 +50,7 @@ static unsigned long compute_checksum(void *addr, unsigned long length) volatile union { uint8_t byte[2]; uint16_t word; - } value; + } chksum; unsigned long sum; unsigned long i;
@@ -72,10 +72,10 @@ static unsigned long compute_checksum(void *addr, unsigned long length) sum = (sum + (sum >> 16)) & 0xFFFF; } } - value.byte[0] = sum & 0xff; - value.byte[1] = (sum >> 8) & 0xff; + chksum.byte[0] = sum & 0xff; + chksum.byte[1] = (sum >> 8) & 0xff;
- return (~value.word) & 0xFFFF; + return (~chksum.word) & 0xFFFF; }
#define for_each_lbrec(head, rec) \ diff --git a/cli_classic.c b/cli_classic.c index d3e7a15..6cb0578 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -150,7 +150,6 @@ int cli_classic(int argc, char *argv[])
if (argc > 1) { /* Yes, print them. */ - int i; printf_debug("The arguments are:\n"); for (i = 1; i < argc; ++i) printf_debug("%s\n", argv[i]); diff --git a/flash.h b/flash.h index 069b903..9a25c70 100644 --- a/flash.h +++ b/flash.h @@ -297,7 +297,6 @@ void internal_delay(int usecs);
extern uint32_t io_base_addr; extern struct pci_access *pacc; -extern struct pci_filter filter; extern struct pci_dev *pcidev_dev; struct pcidev_status { uint16_t vendor_id; diff --git a/hwaccess.h b/hwaccess.h index 2bc2927..0729226 100644 --- a/hwaccess.h +++ b/hwaccess.h @@ -46,12 +46,12 @@ #include <machine/cpufunc.h> #define off64_t off_t #define lseek64 lseek - #define OUTB(x, y) do { u_int tmp = (y); outb(tmp, (x)); } while (0) - #define OUTW(x, y) do { u_int tmp = (y); outw(tmp, (x)); } while (0) - #define OUTL(x, y) do { u_int tmp = (y); outl(tmp, (x)); } while (0) - #define INB(x) __extension__ ({ u_int tmp = (x); inb(tmp); }) - #define INW(x) __extension__ ({ u_int tmp = (x); inw(tmp); }) - #define INL(x) __extension__ ({ u_int tmp = (x); inl(tmp); }) + #define OUTB(x, y) do { u_int outb_tmp = (y); outb(outb_tmp, (x)); } while (0) + #define OUTW(x, y) do { u_int outw_tmp = (y); outw(outw_tmp, (x)); } while (0) + #define OUTL(x, y) do { u_int outl_tmp = (y); outl(outl_tmp, (x)); } while (0) + #define INB(x) __extension__ ({ u_int inb_tmp = (x); inb(inb_tmp); }) + #define INW(x) __extension__ ({ u_int inw_tmp = (x); inw(inw_tmp); }) + #define INL(x) __extension__ ({ u_int inl_tmp = (x); inl(inl_tmp); }) #else #if defined(__DARWIN__) #include <DirectIO/darwinio.h>
On Tue, Jan 12, 2010 at 10:41:41AM +0100, Michael Karcher wrote:
This is not just for fun. We hit a real bug on BSD with the outl macros. The macro variable tmp collided with the tmp from outer scope.
second revision, now also taking care of inb/inw/inl. While that shadowing did not introduce bugs (yet), of course it breaks the build on BSD when -Wshadow is enabled.
Signed-off-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Makefile | 2 +- cbtable.c | 8 ++++---- cli_classic.c | 1 - flash.h | 1 - hwaccess.h | 12 ++++++------ 5 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/Makefile b/Makefile index 423e315..501f65a 100644 --- a/Makefile +++ b/Makefile @@ -25,7 +25,7 @@ INSTALL = install DIFF = diff PREFIX ?= /usr/local MANDIR ?= $(PREFIX)/share/man -CFLAGS ?= -Os -Wall -Werror +CFLAGS ?= -Os -Wall -Werror -Wshadow EXPORTDIR ?= .
OS_ARCH = $(shell uname) diff --git a/cbtable.c b/cbtable.c index 2bc1bfa..2611a62 100644 --- a/cbtable.c +++ b/cbtable.c @@ -50,7 +50,7 @@ static unsigned long compute_checksum(void *addr, unsigned long length) volatile union { uint8_t byte[2]; uint16_t word;
- } value;
- } chksum; unsigned long sum; unsigned long i;
@@ -72,10 +72,10 @@ static unsigned long compute_checksum(void *addr, unsigned long length) sum = (sum + (sum >> 16)) & 0xFFFF; } }
- value.byte[0] = sum & 0xff;
- value.byte[1] = (sum >> 8) & 0xff;
- chksum.byte[0] = sum & 0xff;
- chksum.byte[1] = (sum >> 8) & 0xff;
- return (~value.word) & 0xFFFF;
- return (~chksum.word) & 0xFFFF;
}
#define for_each_lbrec(head, rec) \ diff --git a/cli_classic.c b/cli_classic.c index d3e7a15..6cb0578 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -150,7 +150,6 @@ int cli_classic(int argc, char *argv[])
if (argc > 1) { /* Yes, print them. */
printf_debug("The arguments are:\n"); for (i = 1; i < argc; ++i) printf_debug("%s\n", argv[i]);int i;
diff --git a/flash.h b/flash.h index 069b903..9a25c70 100644 --- a/flash.h +++ b/flash.h @@ -297,7 +297,6 @@ void internal_delay(int usecs);
extern uint32_t io_base_addr; extern struct pci_access *pacc; -extern struct pci_filter filter; extern struct pci_dev *pcidev_dev; struct pcidev_status { uint16_t vendor_id; diff --git a/hwaccess.h b/hwaccess.h index 2bc2927..0729226 100644 --- a/hwaccess.h +++ b/hwaccess.h @@ -46,12 +46,12 @@ #include <machine/cpufunc.h> #define off64_t off_t #define lseek64 lseek
- #define OUTB(x, y) do { u_int tmp = (y); outb(tmp, (x)); } while (0)
- #define OUTW(x, y) do { u_int tmp = (y); outw(tmp, (x)); } while (0)
- #define OUTL(x, y) do { u_int tmp = (y); outl(tmp, (x)); } while (0)
- #define INB(x) __extension__ ({ u_int tmp = (x); inb(tmp); })
- #define INW(x) __extension__ ({ u_int tmp = (x); inw(tmp); })
- #define INL(x) __extension__ ({ u_int tmp = (x); inl(tmp); })
- #define OUTB(x, y) do { u_int outb_tmp = (y); outb(outb_tmp, (x)); } while (0)
- #define OUTW(x, y) do { u_int outw_tmp = (y); outw(outw_tmp, (x)); } while (0)
- #define OUTL(x, y) do { u_int outl_tmp = (y); outl(outl_tmp, (x)); } while (0)
- #define INB(x) __extension__ ({ u_int inb_tmp = (x); inb(inb_tmp); })
- #define INW(x) __extension__ ({ u_int inw_tmp = (x); inw(inw_tmp); })
- #define INL(x) __extension__ ({ u_int inl_tmp = (x); inl(inl_tmp); })
#else #if defined(__DARWIN__) #include <DirectIO/darwinio.h> -- 1.6.5
flashrom mailing list flashrom@flashrom.org http://www.flashrom.org/mailman/listinfo/flashrom
Acked-by: Luc Verhaegen libv@skynet.be
Luc Verhaegen.
Am Dienstag, den 12.01.2010, 16:30 +0100 schrieb Luc Verhaegen:
On Tue, Jan 12, 2010 at 10:41:41AM +0100, Michael Karcher wrote:
This is not just for fun. We hit a real bug on BSD with the outl macros. The macro variable tmp collided with the tmp from outer scope.
second revision, now also taking care of inb/inw/inl. While that shadowing did not introduce bugs (yet), of course it breaks the build on BSD when -Wshadow is enabled.
Signed-off-by: Michael Karcher flashrom@mkarcher.dialup.fu-berlin.de
Acked-by: Luc Verhaegen libv@skynet.be
Thanks, r860.
Regards, Michael Karcher