Re: [Qemu-devel] [PATCH v2 11/13] tcx: remove primitives for non-32-bit surfaces

[Peter Maydell]

On 21 April 2017 at 09:28, Mark Cave-Ayland
<address@hidden> wrote:
> As all surfaces in QEMU are now either shared or 32-bit ARGB regardless of
> the guest depth, remove all non-32-bit primitives from tcx_update_display()
> and consequence their implementation which are no longer required.
>
> Signed-off-by: Mark Cave-Ayland <address@hidden>
> Reviewed-by: Gerd Hoffmann <address@hidden>
> ---
>  hw/display/tcx.c |  126 
> ++++--------------------------------------------------
>  1 file changed, 8 insertions(+), 118 deletions(-)
>
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 0dd007e..6da6dfb 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -140,25 +140,12 @@ static void update_palette_entries(TCXState *s, int 
> start, int end)
>      int i;
>
>      for (i = start; i < end; i++) {
> -        switch (surface_bits_per_pixel(surface)) {
> -        default:
> -        case 8:
> -            s->palette[i] = rgb_to_pixel8(s->r[i], s->g[i], s->b[i]);
> -            break;
> -        case 15:
> -            s->palette[i] = rgb_to_pixel15(s->r[i], s->g[i], s->b[i]);
> -            break;
> -        case 16:
> -            s->palette[i] = rgb_to_pixel16(s->r[i], s->g[i], s->b[i]);
> -            break;
> -        case 32:
> -            if (is_surface_bgr(surface)) {
> -                s->palette[i] = rgb_to_pixel32bgr(s->r[i], s->g[i], s->b[i]);
> -            } else {
> -                s->palette[i] = rgb_to_pixel32(s->r[i], s->g[i], s->b[i]);
> -            }
> -            break;
> +        if (is_surface_bgr(surface)) {
> +            s->palette[i] = rgb_to_pixel32bgr(s->r[i], s->g[i], s->b[i]);
> +        } else {
> +            s->palette[i] = rgb_to_pixel32(s->r[i], s->g[i], s->b[i]);
>          }
> +        break;
>      }

Something's gone wrong here. Coverity points out that we now
have a for() loop that only executes once, because it ends
with a "break" statement. Looks like the 'break' from inside
the 'case 32:' was retained when the switch was deleted,
and it should just be deleted?

thanks
-- PMM