Re: [PULL 17/27] ati-vga: Fix colors when frame buffer endianness does not match host

[Peter Maydell <peter.maydell@linaro.org>]

On Mon, 23 Mar 2026 at 16:54, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> From: BALATON Zoltan <balaton@eik.bme.hu>
>
> When writing pixels we have to take into account if the frame buffer
> endianness matches the host endianness or we need to swap to correct
> endianness. This caused wrong colors e.g. with PPC Linux guest that
> uses big endian frame buffer when running on little endian host.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> Tested-by: Chad Jablonski <chad@jablonski.xyz>
> Reviewed-by: Chad Jablonski <chad@jablonski.xyz>
> Message-ID: <759ed5e3b019cce94e9a4ef003f1fc2e0cea2ec1.1774110169.git.balaton@eik.bme.hu>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Hi; Coverity flags this as possibly-wrong code (CID 1645969:

> @@ -89,6 +90,7 @@ static void setup_2d_blt_ctx(const ATIVGAState *s, ATI2DCtx *ctx)
>      ctx->host_data_active = s->host_data.active;
>      ctx->left_to_right = s->regs.dp_cntl & DST_X_LEFT_TO_RIGHT;
>      ctx->top_to_bottom = s->regs.dp_cntl & DST_Y_TOP_TO_BOTTOM;
> +    ctx->need_swap = HOST_BIG_ENDIAN != s->vga.big_endian_fb ? true : false;

The issue is that != is higher priority than ?:, and people often
write "X != A ? B : C" when they wanted "X != (A ? B : C)" but they
get "(X != A) ? B : C". Either way, using parentheses helps clarify
for readers.

The priority is actually right for this particular case, but the
expression is unnecessarily complex: we could write more simply:

  ctx->need_swap = (HOST_BIG_ENDIAN != s->vga.big_endian_fb);

I've marked the issue in Coverity as false-positive, but we might
consider changing the code also.

-- PMM