[PATCH v2] hw/i386/pc: Remove vmport value assertion

[PATCH v2] hw/i386/pc: Remove vmport value assertion

[Kamil Szczęk]

There is no need for this assertion here, as we only use vmport value
for equality/inequality checks. This was originally prompted by the
following Coverity report:
 >>> CID 1559533:  Integer handling issues (CONSTANT_EXPRESSION_RESULT)
 >>> "pcms->vmport >= 0" is always true regardless of the values of
 >>> its operands. This occurs as the logical first operand of "&&".

Signed-off-by: Kamil Szczęk <kamil@szczek.dev>
Reported-By: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 7779c88a91..5302fd96b4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1221,7 +1221,6 @@ void pc_basic_device_init(struct PCMachineState *pcms,
         isa_realize_and_unref(pcms->pcspk, isa_bus, &error_fatal);
     }
 
-    assert(pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX);
     if (pcms->vmport == ON_OFF_AUTO_AUTO) {
         pcms->vmport = (xen_enabled() || !pcms->i8042_enabled)
             ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;
-- 
2.45.0

Re: [PATCH v2] hw/i386/pc: Remove vmport value assertion

[Richard Henderson]

On 8/21/24 09:11, Kamil Szczęk wrote:
> There is no need for this assertion here, as we only use vmport value
> for equality/inequality checks. This was originally prompted by the
> following Coverity report:
>   >>> CID 1559533:  Integer handling issues (CONSTANT_EXPRESSION_RESULT)
>   >>> "pcms->vmport >= 0" is always true regardless of the values of
>   >>> its operands. This occurs as the logical first operand of "&&".
> 
> Signed-off-by: Kamil Szczęk <kamil@szczek.dev>
> Reported-By: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/i386/pc.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 7779c88a91..5302fd96b4 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1221,7 +1221,6 @@ void pc_basic_device_init(struct PCMachineState *pcms,
>           isa_realize_and_unref(pcms->pcspk, isa_bus, &error_fatal);
>       }
>   
> -    assert(pcms->vmport >= 0 && pcms->vmport < ON_OFF_AUTO__MAX);
>       if (pcms->vmport == ON_OFF_AUTO_AUTO) {
>           pcms->vmport = (xen_enabled() || !pcms->i8042_enabled)
>               ? ON_OFF_AUTO_OFF : ON_OFF_AUTO_ON;

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v2] hw/i386/pc: Remove vmport value assertion

[Kamil Szczęk]

On Wednesday, August 21st, 2024 at 01:11, Kamil Szczęk <kamil@szczek.dev> wrote:
> There is no need for this assertion here, as we only use vmport value
> for equality/inequality checks. This was originally prompted by the
> following Coverity report:
>  >>> CID 1559533: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
>  >>> "pcms->vmport >= 0" is always true regardless of the values of
>  >>> its operands. This occurs as the logical first operand of "&&".
> 
> Signed-off-by: Kamil Szczęk <kamil@szczek.dev>
> Reported-By: Philippe Mathieu-Daudé <philmd@linaro.org>

Hi, just checking in to see if this needs any additional work.

Re: [PATCH v2] hw/i386/pc: Remove vmport value assertion

[Peter Maydell]

On Mon, 16 Sept 2024 at 15:18, Kamil Szczęk <kamil@szczek.dev> wrote:
>
> On Wednesday, August 21st, 2024 at 01:11, Kamil Szczęk <kamil@szczek.dev> wrote:
> > There is no need for this assertion here, as we only use vmport value
> > for equality/inequality checks. This was originally prompted by the
> > following Coverity report:
> >  >>> CID 1559533: Integer handling issues (CONSTANT_EXPRESSION_RESULT)
> >  >>> "pcms->vmport >= 0" is always true regardless of the values of
> >  >>> its operands. This occurs as the logical first operand of "&&".
> >
> > Signed-off-by: Kamil Szczęk <kamil@szczek.dev>
> > Reported-By: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Hi, just checking in to see if this needs any additional work.

No, it's fine -- looks like we just failed to pick up the
patch. Sorry about that. We'll get it into 9.2.

thanks
-- PMM