Skip to contentSkip to conversation

vga: squash build error in vga_update_memory_access()

PATCH In review Avi Kivity 3 participants 8 messages Aug 24, 2011 – Aug 25, 2011

lore
Share & tools
lore

Avi Kivity

author
#0 lore raw

PATCH 1/1 vga: squash build error in vga_update_memory_access()

Newer gcc complains that base and size may be used uninitialized, even though it is clearly a false warning. Silence the warning by indicating to gcc that the code path triggering the warning cannot happen.

---
 hw/vga.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
hw/vga.c +2 -0
index 851fd68..b74e6e8 100644
@@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s)
             base = 0xb8000;
             size = 0x8000;
             break;
+        default:
+            abort();
         }
         region = g_malloc(sizeof(*region));
         memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);
Signature · 1 line
1.7.5.3
#1 lore raw
Quoted 2 lines · Avi Kivity

Newer gcc complains that base and size may be used uninitialized, even though it is clearly a false warning.

Mmh, gcc is getting dumber again.

Quoted 13 lines

Silence the warning by indicating to gcc that
the code path triggering the warning cannot happen.

---
 hw/vga.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
hw/vga.c +0 -0
index 851fd68..b74e6e8 100644

hw/vga.c static void vga_update_memory_access(VGACommonState *s) · line 186

Quoted 8 lines
hw/vga.c +2 -0
@@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s)
             base = 0xb8000;
             size = 0x8000;
             break;
+        default:
+            abort();
         }
         region = g_malloc(sizeof(*region));
         memory_region_init_alias(region, "vga.chain4", &s->vram, offset, size);

...or just make the last case default?

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
author
#2 lore raw
Quoted 11 lines · Jan Kiszka
Quoted 10 lines

+++ b/hw/vga.c
@@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s)

              base = 0xb8000;
              size = 0x8000;
              break;
 +        default:
 +            abort();
          }
          region = g_malloc(sizeof(*region));
          memory_region_init_alias(region, "vga.chain4",&s->vram, offset, size);
Quoted 2 lines

...or just make the last case default?

No reason to make the code unobvious in this path, IMO. Eventually gcc will be able to drop the 4/5 bytes this patch adds to the object code.

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
#3 lore raw
Quoted 12 lines · Avi Kivity
Quoted 11 lines · Jan Kiszka
Quoted 10 lines

+++ b/hw/vga.c
@@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s)

              base = 0xb8000;
              size = 0x8000;
              break;
 +        default:
 +            abort();
          }
          region = g_malloc(sizeof(*region));
          memory_region_init_alias(region, "vga.chain4",&s->vram, offset, size);
Quoted 5 lines
Quoted 2 lines

...or just make the last case default?

No reason to make the code unobvious in this path, IMO. Eventually gcc will be able to drop the 4/5 bytes this patch adds to the object code.

hw/vga.c +1 -0
index 851fd68..125fb29 100644
@@ -176,6 +176,7 @@ static void vga_update_memory_access(VGACommonState *s)
             size = 0x8000;
             break;
         case 3:
+        default:
             base = 0xb8000;
             size = 0x8000;
             break;

...is fairly common and well readable IMHO.

Jan

Signature · 2 lines
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
author
#4 lore raw
Quoted 32 lines · Jan Kiszka
Quoted 17 lines · Avi Kivity
Quoted 11 lines · Jan Kiszka
Quoted 10 lines
  +++ b/hw/vga.c
  @@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s)
               base = 0xb8000;
               size = 0x8000;
               break;
  +        default:
  +            abort();
           }
           region = g_malloc(sizeof(*region));
           memory_region_init_alias(region, "vga.chain4",&s->vram, offset, size);
Quoted 2 lines

...or just make the last case default?

No reason to make the code unobvious in this path, IMO. Eventually gcc will be able to drop the 4/5 bytes this patch adds to the object code.

hw/vga.c +1 -0
index 851fd68..125fb29 100644
@@ -176,6 +176,7 @@ static void vga_update_memory_access(VGACommonState *s)
              size = 0x8000;
              break;
          case 3:
+        default:
              base = 0xb8000;
              size = 0x8000;
              break;
Quoted 3 lines

...is fairly common and well readable IMHO.

Let's let the maintainers pick the one they like.

Signature · 2 lines
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
#5 lore raw
In reply to Jan Kiszka
Quoted 32 lines · Jan Kiszka
Quoted 17 lines · Avi Kivity
Quoted 11 lines · Jan Kiszka
Quoted 10 lines
  +++ b/hw/vga.c
  @@ -179,6 +179,8 @@ static void vga_update_memory_access(VGACommonState *s)
               base = 0xb8000;
               size = 0x8000;
               break;
  +        default:
  +            abort();
           }
           region = g_malloc(sizeof(*region));
           memory_region_init_alias(region, "vga.chain4",&s->vram, offset, size);
Quoted 2 lines

...or just make the last case default?

No reason to make the code unobvious in this path, IMO. Eventually gcc will be able to drop the 4/5 bytes this patch adds to the object code.

hw/vga.c +1 -0
index 851fd68..125fb29 100644
@@ -176,6 +176,7 @@ static void vga_update_memory_access(VGACommonState *s)
              size = 0x8000;
              break;
          case 3:
+        default:
              base = 0xb8000;
              size = 0x8000;
              break;
Quoted 1 line

...is fairly common and well readable IMHO.

And is less likely to be considered by someone to be a guest triggerable abort().

Can you send the patch as a top-level one with a Signed-off-by?

Regards,

Anthony Liguori

Quoted 3 lines

Jan

#6 lore raw

PATCH 1/1 vga: Silence bogus gcc warning about uninitialized variables

Some gcc versions do not properly detect that all possible cases are covered and base and size are always initialized. Please gcc by defining a pseudo default case.

---
 hw/vga.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
hw/vga.c +1 -0
index 851fd68..125fb29 100644
@@ -176,6 +176,7 @@ static void vga_update_memory_access(VGACommonState *s)
             size = 0x8000;
             break;
         case 3:
+        default:
             base = 0xb8000;
             size = 0x8000;
             break;
Signature · 1 line
1.7.3.4
#7 lore raw

Re: [Qemu-devel] [PATCH] vga: Silence bogus gcc warning about uninitialized variables

Quoted 5 lines · Jan Kiszka

Some gcc versions do not properly detect that all possible cases are covered and base and size are always initialized. Please gcc by defining a pseudo default case.

Applied. Thanks.

Regards,

Anthony Liguori

Quoted 16 lines
---
  hw/vga.c |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
hw/vga.c +1 -0
index 851fd68..125fb29 100644
@@ -176,6 +176,7 @@ static void vga_update_memory_access(VGACommonState *s)
              size = 0x8000;
              break;
          case 3:
+        default:
              base = 0xb8000;
              size = 0x8000;
              break;