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? > 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]>
