Skip to contentSkip to conversation

ide: fix enum comparison for gcc 4.7

PATCH Superseded John Snow 3 participants 5 messages Sep 20, 2017

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

John Snow

author
#0 lore raw

PATCH 1/1 ide: fix enum comparison for gcc 4.7

Apparently GCC gets bent over comparing enum values against zero. Replace the conditional with something less readable.

Signed-off-by John Snow <[email protected]>
---
 hw/ide/core.c             | 2 +-
 include/hw/ide/internal.h | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)
hw/ide/core.c +1 -1
index a19bd90..d63eb4a 100644
@@ -68,7 +68,7 @@ const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {
 
 static const char *IDE_DMA_CMD_str(enum ide_dma_cmd enval)
 {
-    if (enval >= IDE_DMA__BEGIN && enval < IDE_DMA__COUNT) {
+    if ((unsigned)enval < IDE_DMA__COUNT) {
         return IDE_DMA_CMD_lookup[enval];
     }
     return "DMA UNKNOWN CMD";
include/hw/ide/internal.h +1 -2
index 180e00e..e641012 100644
@@ -333,8 +333,7 @@ struct unreported_events {
 };
 
 enum ide_dma_cmd {
-    IDE_DMA__BEGIN = 0,
-    IDE_DMA_READ = IDE_DMA__BEGIN,
+    IDE_DMA_READ = 0,
     IDE_DMA_WRITE,
     IDE_DMA_TRIM,
     IDE_DMA_ATAPI,
Signature · 1 line
2.9.5
Reviewed-by on v1 1/1
#1 lore raw
Quoted 8 lines · John Snow

Apparently GCC gets bent over comparing enum values against zero. Replace the conditional with something less readable.

Signed-off-by John Snow <[email protected]>
---
 hw/ide/core.c             | 2 +-
 include/hw/ide/internal.h | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

Unfortunate that the compiler conspires against aesthetics, but such is life.

Reviewed-by Eric Blake <[email protected]>
Signature · 3 lines
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org
#2 lore raw
In reply to John Snow
Quoted 28 lines · John Snow

Apparently GCC gets bent over comparing enum values against zero. Replace the conditional with something less readable.

Signed-off-by John Snow <[email protected]>
---
 hw/ide/core.c             | 2 +-
 include/hw/ide/internal.h | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)
hw/ide/core.c +1 -1
index a19bd90..d63eb4a 100644
@@ -68,7 +68,7 @@ const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {
 
 static const char *IDE_DMA_CMD_str(enum ide_dma_cmd enval)
 {
-    if (enval >= IDE_DMA__BEGIN && enval < IDE_DMA__COUNT) {
+    if ((unsigned)enval < IDE_DMA__COUNT) {
         return IDE_DMA_CMD_lookup[enval];
     }
     return "DMA UNKNOWN CMD";
include/hw/ide/internal.h +0 -0
index 180e00e..e641012 100644

include/hw/ide/internal.h struct unreported_events { · line 339

Quoted 9 lines
include/hw/ide/internal.h +1 -2
@@ -333,8 +333,7 @@ struct unreported_events {
 };
 
 enum ide_dma_cmd {
-    IDE_DMA__BEGIN = 0,
-    IDE_DMA_READ = IDE_DMA__BEGIN,
+    IDE_DMA_READ = 0,
     IDE_DMA_WRITE,
     IDE_DMA_TRIM,
     IDE_DMA_ATAPI,

Really close - it fixes the error in hw/ide/core.c but then I see a similar error a bit later in hw/ide/ahci.c:

cc -I/home/build/src/qemu/git/qemu/hw/ide -Ihw/ide -I/home/build/src/qemu/git/qemu/tcg -I/home/build/src/qemu/git/qemu/tcg/i386 -I/home/build/src/qemu/git/qemu/linux-headers -I/home/build/src/qemu/git/qemu/linux-headers -I. -I/home/build/src/qemu/git/qemu -I/home/build/src/qemu/git/qemu/accel/tcg -I/home/build/src/qemu/git/qemu/include -I/usr/include/pixman-1 -I/home/build/src/qemu/git/qemu/dtc/libfdt -Werror -pthread -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all -I/usr/include/p11-kit-1
-I/usr/include/libpng12 -I/home/build/src/qemu/git/qemu/tests -MMD -MP
-MT hw/ide/ahci.o -MF hw/ide/ahci.d -O2 -U_FORTIFY_SOURCE
-D_FORTIFY_SOURCE=2 -g -c -o hw/ide/ahci.o hw/ide/ahci.c
hw/ide/ahci.c: In function ‘ahci_trigger_irq’:
hw/ide/ahci.c:187:5: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
cc1: all warnings being treated as errors
make: *** [hw/ide/ahci.o] Error 1

ATB,

Mark.

author
#3 lore raw
Quoted 70 lines · Mark Cave-Ayland
Quoted 28 lines · John Snow

Apparently GCC gets bent over comparing enum values against zero. Replace the conditional with something less readable.

Signed-off-by John Snow <[email protected]>
---
 hw/ide/core.c             | 2 +-
 include/hw/ide/internal.h | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)
hw/ide/core.c +1 -1
index a19bd90..d63eb4a 100644
@@ -68,7 +68,7 @@ const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {
 
 static const char *IDE_DMA_CMD_str(enum ide_dma_cmd enval)
 {
-    if (enval >= IDE_DMA__BEGIN && enval < IDE_DMA__COUNT) {
+    if ((unsigned)enval < IDE_DMA__COUNT) {
         return IDE_DMA_CMD_lookup[enval];
     }
     return "DMA UNKNOWN CMD";
include/hw/ide/internal.h +0 -0
index 180e00e..e641012 100644

include/hw/ide/internal.h struct unreported_events { · line 339

Quoted 9 lines
include/hw/ide/internal.h +1 -2
@@ -333,8 +333,7 @@ struct unreported_events {
 };
 
 enum ide_dma_cmd {
-    IDE_DMA__BEGIN = 0,
-    IDE_DMA_READ = IDE_DMA__BEGIN,
+    IDE_DMA_READ = 0,
     IDE_DMA_WRITE,
     IDE_DMA_TRIM,
     IDE_DMA_ATAPI,

Really close - it fixes the error in hw/ide/core.c but then I see a similar error a bit later in hw/ide/ahci.c:

cc -I/home/build/src/qemu/git/qemu/hw/ide -Ihw/ide

(quoted diff) +0 -19
-I/home/build/src/qemu/git/qemu/tcg
-I/home/build/src/qemu/git/qemu/tcg/i386
-I/home/build/src/qemu/git/qemu/linux-headers
-I/home/build/src/qemu/git/qemu/linux-headers -I.
-I/home/build/src/qemu/git/qemu
-I/home/build/src/qemu/git/qemu/accel/tcg
-I/home/build/src/qemu/git/qemu/include -I/usr/include/pixman-1
-I/home/build/src/qemu/git/qemu/dtc/libfdt -Werror -pthread
-I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include
-m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings
-Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv
-Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wold-style-declaration -Wold-style-definition -Wtype-limits
-fstack-protector-all -I/usr/include/p11-kit-1
-I/usr/include/libpng12   -I/home/build/src/qemu/git/qemu/tests -MMD -MP
-MT hw/ide/ahci.o -MF hw/ide/ahci.d -O2 -U_FORTIFY_SOURCE
-D_FORTIFY_SOURCE=2 -g   -c -o hw/ide/ahci.o hw/ide/ahci.c

hw/ide/ahci.c: In function ‘ahci_trigger_irq’:
hw/ide/ahci.c:187:5: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
cc1: all warnings being treated as errors
make: *** [hw/ide/ahci.o] Error 1

Quoted 4 lines

ATB,

Mark.

Man, what's with your compiler? ...

OK, let's try:

hw/ide/ahci.c +1 -1
index 24c65df..32d1296 100644
@@ -184,7 +184,7 @@ static void ahci_check_irq(AHCIState *s)
 static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
                              enum AHCIPortIRQ irqbit)
 {
-    g_assert(irqbit >= 0 && irqbit < 32);
+    g_assert((unsigned)irqbit < 32);
     uint32_t irq = 1U << irqbit;
     uint32_t irqstat = d->port_regs.irq_stat | irq;

I can't remember immediately if I have more spots that might cause a ruckus for you.

Tested-by
#4 lore raw
Quoted 94 lines · John Snow
Quoted 70 lines · Mark Cave-Ayland
Quoted 28 lines · John Snow

Apparently GCC gets bent over comparing enum values against zero. Replace the conditional with something less readable.

Signed-off-by John Snow <[email protected]>
---
 hw/ide/core.c             | 2 +-
 include/hw/ide/internal.h | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)
hw/ide/core.c +1 -1
index a19bd90..d63eb4a 100644
@@ -68,7 +68,7 @@ const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {
 
 static const char *IDE_DMA_CMD_str(enum ide_dma_cmd enval)
 {
-    if (enval >= IDE_DMA__BEGIN && enval < IDE_DMA__COUNT) {
+    if ((unsigned)enval < IDE_DMA__COUNT) {
         return IDE_DMA_CMD_lookup[enval];
     }
     return "DMA UNKNOWN CMD";
include/hw/ide/internal.h +0 -0
index 180e00e..e641012 100644

include/hw/ide/internal.h struct unreported_events { · line 339

Quoted 9 lines
include/hw/ide/internal.h +1 -2
@@ -333,8 +333,7 @@ struct unreported_events {
 };
 
 enum ide_dma_cmd {
-    IDE_DMA__BEGIN = 0,
-    IDE_DMA_READ = IDE_DMA__BEGIN,
+    IDE_DMA_READ = 0,
     IDE_DMA_WRITE,
     IDE_DMA_TRIM,
     IDE_DMA_ATAPI,

Really close - it fixes the error in hw/ide/core.c but then I see a similar error a bit later in hw/ide/ahci.c:

cc -I/home/build/src/qemu/git/qemu/hw/ide -Ihw/ide

(quoted diff) +0 -19
-I/home/build/src/qemu/git/qemu/tcg
-I/home/build/src/qemu/git/qemu/tcg/i386
-I/home/build/src/qemu/git/qemu/linux-headers
-I/home/build/src/qemu/git/qemu/linux-headers -I.
-I/home/build/src/qemu/git/qemu
-I/home/build/src/qemu/git/qemu/accel/tcg
-I/home/build/src/qemu/git/qemu/include -I/usr/include/pixman-1
-I/home/build/src/qemu/git/qemu/dtc/libfdt -Werror -pthread
-I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include
-m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE
-Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings
-Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv
-Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs
-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
-Wold-style-declaration -Wold-style-definition -Wtype-limits
-fstack-protector-all -I/usr/include/p11-kit-1
-I/usr/include/libpng12   -I/home/build/src/qemu/git/qemu/tests -MMD -MP
-MT hw/ide/ahci.o -MF hw/ide/ahci.d -O2 -U_FORTIFY_SOURCE
-D_FORTIFY_SOURCE=2 -g   -c -o hw/ide/ahci.o hw/ide/ahci.c

hw/ide/ahci.c: In function ‘ahci_trigger_irq’:
hw/ide/ahci.c:187:5: error: comparison of unsigned expression >= 0 is always true [-Werror=type-limits]
cc1: all warnings being treated as errors
make: *** [hw/ide/ahci.o] Error 1

Quoted 4 lines

ATB,

Mark.

Man, what's with your compiler? ...

OK, let's try:

hw/ide/ahci.c +1 -1
index 24c65df..32d1296 100644
@@ -184,7 +184,7 @@ static void ahci_check_irq(AHCIState *s)
 static void ahci_trigger_irq(AHCIState *s, AHCIDevice *d,
                              enum AHCIPortIRQ irqbit)
 {
-    g_assert(irqbit >= 0 && irqbit < 32);
+    g_assert((unsigned)irqbit < 32);
     uint32_t irq = 1U << irqbit;
     uint32_t irqstat = d->port_regs.irq_stat | irq;
Quoted 2 lines

I can't remember immediately if I have more spots that might cause a ruckus for you.

Nope, that's all folks! Combining these two together fixes the build for me again:

Tested-by Mark Cave-Ayland <[email protected]>

ATB,

Mark.