| Message ID | 4E54F5E9.1020104@siemens.com |
|---|---|
| State | New |
| Headers | show |
On 08/24/2011 04:00 PM, Jan Kiszka wrote: > On 2011-08-24 14:58, Avi Kivity wrote: > > On 08/24/2011 03:46 PM, Jan Kiszka wrote: > >>> +++ 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); > >> > >> ...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. > > diff --git a/hw/vga.c b/hw/vga.c > index 851fd68..125fb29 100644 > --- a/hw/vga.c > +++ b/hw/vga.c > @@ -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. > > Let's let the maintainers pick the one they like.
On 08/24/2011 08:00 AM, Jan Kiszka wrote: > On 2011-08-24 14:58, Avi Kivity wrote: >> On 08/24/2011 03:46 PM, Jan Kiszka wrote: >>>> +++ 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); >>> >>> ...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. > > diff --git a/hw/vga.c b/hw/vga.c > index 851fd68..125fb29 100644 > --- a/hw/vga.c > +++ b/hw/vga.c > @@ -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. 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 > > Jan >
diff --git a/hw/vga.c b/hw/vga.c index 851fd68..125fb29 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -176,6 +176,7 @@ static void vga_update_memory_access(VGACommonState *s) size = 0x8000; break; case 3: + default: base = 0xb8000; size = 0x8000; break;