On Wed, 31 Jul 2024 at 15:47, Markus Armbruster <[email protected]> wrote: > > Peter Maydell <[email protected]> writes: > > > In ide_atapi_cmd_reply_end() we calculate a 16-bit size, and then > > assign its two halves to s->lcyl and s->hcyl like this: > > > > s->lcyl = size; > > s->hcyl = size >> 8; > > > > Coverity warns that the first line here can overflow the > > 8-bit s->lcyl variable. This is true, and in this case we're > > deliberately only after the low 8 bits of the value. The > > code is clearer to both humans and Coverity if we're explicit > > that we only wanted the low 8 bits, though. > > > > Resolves?
Whoops. Resolves: Coverity CID 1547621 > > Signed-off-by: Peter Maydell <[email protected]> > > --- > > hw/ide/atapi.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c > > index fcb6cca1573..e82959dc2d3 100644 > > --- a/hw/ide/atapi.c > > +++ b/hw/ide/atapi.c > > @@ -265,7 +265,7 @@ void ide_atapi_cmd_reply_end(IDEState *s) > /* a new transfer is needed */ > s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO; > ide_bus_set_irq(s->bus); > byte_count_limit = atapi_byte_count_limit(s); > trace_ide_atapi_cmd_reply_end_bcl(s, byte_count_limit); > size = s->packet_transfer_size; > if (size > byte_count_limit) { > /* byte count limit must be even if this case */ > if (byte_count_limit & 1) > > byte_count_limit--; > > size = byte_count_limit; > > } > > - s->lcyl = size; > > + s->lcyl = size & 0xff; > > s->hcyl = size >> 8; > > s->elementary_transfer_size = size; > > /* we cannot transmit more than one sector at a time */ > > @size is int. I wonder why it's fine with size >> 8... ah, > atapi_byte_count_limit() returns uint16_t, and Coverity is smart enough > to data-flow this via @byte_count_limit into @size. > > Reviewed-by: Markus Armbruster <[email protected]> thanks -- PMM
