Re: [PATCH] hw/i386/pc: Fix enum value assertion

[Kamil Szczęk]

On Wednesday, August 21st, 2024 at 00:59, Richard Henderson 
<richard.henderson@linaro.org> wrote:
> On 8/21/24 08:52, Kamil Szczęk wrote:
> 
> > Coverity reported:
> > 
> > > > > 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 | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 7779c88a91..523dfe3f3f 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1221,7 +1221,7 @@ 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);
> > + assert(pcms->vmport < ON_OFF_AUTO__MAX);
> 
> 
> Given the usage here
> 
> > if (pcms->vmport == ON_OFF_AUTO_AUTO) {
> 
> 
> and later, here
> 
> > pc_superio_init(isa_bus, create_fdctrl, pcms->i8042_enabled,
> > pcms->vmport != ON_OFF_AUTO_ON, &error_fatal);
> 
> 
> absolutely nothing goes off the rails no matter the value of vmport.
> 
> It is not used to index an array, which might be out of bounds.
> It it not a security issue.
> There's no need or benefit for the assert at all.
> 

Agreed, v2 posted.