ati-vga Coverity fixes

[PATCH 1/2] ati-vga: Silence warning about operator precedence

Posted by BALATON Zoltan 2 days, 4 hours ago

Coverity in CID 1645969 warns about possible operator precendence
issue which is a false positive in this case but simplify the
expression to silence the warning.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/display/ati_2d.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
index 23527b2c50..9baf6ff37b 100644
--- a/hw/display/ati_2d.c
+++ b/hw/display/ati_2d.c
@@ -91,7 +91,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;
+    ctx->need_swap = (HOST_BIG_ENDIAN != s->vga.big_endian_fb);
     ctx->frgd_clr = s->regs.dp_brush_frgd_clr;
     ctx->palette = s->vga.palette;
     ctx->dst_offset = s->regs.dst_offset;
-- 
2.41.3

Re: [PATCH 1/2] ati-vga: Silence warning about operator precedence

Posted by Philippe Mathieu-Daudé 1 day, 12 hours ago

On 30/3/26 23:09, BALATON Zoltan wrote:
> Coverity in CID 1645969 warns about possible operator precendence
> issue which is a false positive in this case but simplify the
> expression to silence the warning.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/display/ati_2d.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
> index 23527b2c50..9baf6ff37b 100644
> --- a/hw/display/ati_2d.c
> +++ b/hw/display/ati_2d.c
> @@ -91,7 +91,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;
> +    ctx->need_swap = (HOST_BIG_ENDIAN != s->vga.big_endian_fb);

Do we really need the parenthesis?

>       ctx->frgd_clr = s->regs.dp_brush_frgd_clr;
>       ctx->palette = s->vga.palette;
>       ctx->dst_offset = s->regs.dst_offset;

Re: [PATCH 1/2] ati-vga: Silence warning about operator precedence

Posted by BALATON Zoltan 1 day, 11 hours ago

On Tue, 31 Mar 2026, Philippe Mathieu-Daudé wrote:
> On 30/3/26 23:09, BALATON Zoltan wrote:
>> Coverity in CID 1645969 warns about possible operator precendence
>> issue which is a false positive in this case but simplify the
>> expression to silence the warning.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/display/ati_2d.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
>> index 23527b2c50..9baf6ff37b 100644
>> --- a/hw/display/ati_2d.c
>> +++ b/hw/display/ati_2d.c
>> @@ -91,7 +91,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;
>> +    ctx->need_swap = (HOST_BIG_ENDIAN != s->vga.big_endian_fb);
>
> Do we really need the parenthesis?

Should not be needed but it may be more readable and Coverity warned about 
the original that did not need parenthesis either so it seems the safest 
to write it this way.

Regards,
BALATON Zoltan

>>       ctx->frgd_clr = s->regs.dp_brush_frgd_clr;
>>       ctx->palette = s->vga.palette;
>>       ctx->dst_offset = s->regs.dst_offset;
>
>

Re: [PATCH 1/2] ati-vga: Silence warning about operator precedence

Posted by Peter Maydell 1 day, 17 hours ago

On Mon, 30 Mar 2026 at 22:09, BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> Coverity in CID 1645969 warns about possible operator precendence
> issue which is a false positive in this case but simplify the
> expression to silence the warning.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/display/ati_2d.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c
> index 23527b2c50..9baf6ff37b 100644
> --- a/hw/display/ati_2d.c
> +++ b/hw/display/ati_2d.c
> @@ -91,7 +91,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;
> +    ctx->need_swap = (HOST_BIG_ENDIAN != s->vga.big_endian_fb);
>      ctx->frgd_clr = s->regs.dp_brush_frgd_clr;
>      ctx->palette = s->vga.palette;
>      ctx->dst_offset = s->regs.dst_offset;
> --

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM