Hi Stefan, this patch includes [flashrom] [PATCH] Fix undefined behavior in OS defines Is that intentional?
One minor nitpick: Using "#ifdef IS_LINUX" will be true on all platforms with that change. Admittedly it probably was undefined behaviour before, so this is an improvement. I wonder if we should add a comment before the IS_ and USE_ #defines to tell people to always use them with #if and never with if defined() or #ifdef. Acked-by: Carl-Daniel Hailfinger <[email protected]> Regards, Carl-Daniel On 02.12.2016 01:49, Stefan Tauner wrote: > The macro below (as used it was used previously) would produce > undefined behavior when used as expression in an > #if preprocessor directive: > #define IS_LINUX (defined(__gnu_linux__) || defined(__linux__)) > > This patch replaces all such statements with a more verbose but > well-defined #if/#define x 1/#else/#define x 0/#endif > > Found by clang (warning introduced in r258128), reported by Idwer. > > Signed-off-by: Stefan Tauner <[email protected]> > --- > hwaccess.c | 18 +++++++++++++++--- > platform.h | 19 +++++++++++++++---- > 2 files changed, 30 insertions(+), 7 deletions(-) > > diff --git a/hwaccess.c b/hwaccess.c > index 1901ee6..80852e7 100644 > --- a/hwaccess.c > +++ b/hwaccess.c > @@ -37,9 +37,21 @@ > #error "Unknown operating system" > #endif > > -#define USE_IOPL (IS_LINUX || IS_MACOSX || defined(__NetBSD__) || > defined(__OpenBSD__)) > -#define USE_DEV_IO (defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || > defined(__DragonFly__)) > -#define USE_IOPERM (defined(__gnu_hurd__)) > +#if (IS_LINUX || IS_MACOSX || defined(__NetBSD__) || defined(__OpenBSD__)) > + #define USE_IOPL (1) > +#else > + #define USE_IOPL (0) > +#endif > +#if (defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || > defined(__DragonFly__)) > + #define USE_DEV_IO (1) > +#else > + #define USE_DEV_IO (0) > +#endif > +#if (defined(__gnu_hurd__)) > + #define USE_IOPERM (1) > +#else > + #define USE_IOPERM (0) > +#endif > > #if USE_IOPERM > #include <sys/io.h> > diff --git a/platform.h b/platform.h > index c5a52ef..2785753 100644 > --- a/platform.h > +++ b/platform.h > @@ -24,10 +24,21 @@ > #ifndef __PLATFORM_H__ > #define __PLATFORM_H__ 1 > > -// Helper defines for operating systems > -#define IS_LINUX (defined(__gnu_linux__) || defined(__linux__)) > -#define IS_MACOSX (defined(__APPLE__) && defined(__MACH__)) /* yes, both. > */ > -#define IS_WINDOWS (defined(_WIN32) || defined(_WIN64) || > defined(__WIN32__) || defined(__WINDOWS__)) > +#if (defined(__gnu_linux__) || defined(__linux__)) > + #define IS_LINUX (1) > +#else > + #define IS_LINUX (0) > +#endif > +#if (defined(__APPLE__) && defined(__MACH__)) /* yes, both. */ > + #define IS_MACOSX (1) > +#else > + #define IS_MACOSX (0) > +#endif > +#if (defined(_WIN32) || defined(_WIN64) || defined(__WIN32__) || > defined(__WINDOWS__)) > + #define IS_WINDOWS (1) > +#else > + #define IS_WINDOWS (0) > +#endif > > // Likewise for target architectures > #if defined (__i386__) || defined (__x86_64__) || defined(__amd64__) _______________________________________________ flashrom mailing list [email protected] https://www.flashrom.org/mailman/listinfo/flashrom
