On Thu, 17 Jul 2025 at 12:51, Philippe Mathieu-Daudé <phi...@linaro.org> wrote: > > Hi Mark, > > On 15/7/25 08:19, Philippe Mathieu-Daudé wrote: > > From: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> > > > > In the cases where mixed DMA/non-DMA transfers are used or no data is > > available, it is possible for the calculated transfer length to be zero. > > Only call the dma_memory_read function where the transfer length is > > non-zero to avoid invoking the DMA engine for a zero length transfer > > which can have side-effects (along with generating additional tracing > > noise). > > > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> > > Reviewed-by: Philippe Mathieu-Daudé <phi...@linaro.org> > > Tested-by: Philippe Mathieu-Daudé <phi...@linaro.org> > > Message-ID: <20250711204636.542964-5-mark.cave-ayl...@ilande.co.uk> > > Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org> > > --- > > hw/scsi/esp.c | 20 +++++++++++++------- > > 1 file changed, 13 insertions(+), 7 deletions(-) > > > > diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c > > index 62ba4061492..ec9fcbeddf4 100644 > > --- a/hw/scsi/esp.c > > +++ b/hw/scsi/esp.c > > @@ -487,8 +487,10 @@ static void esp_do_dma(ESPState *s) > > case STAT_MO: > > if (s->dma_memory_read) { > > len = MIN(len, fifo8_num_free(&s->cmdfifo)); > > - s->dma_memory_read(s->dma_opaque, buf, len); > > - esp_set_tc(s, esp_get_tc(s) - len); > > + if (len) { > > + s->dma_memory_read(s->dma_opaque, buf, len); > > + esp_set_tc(s, esp_get_tc(s) - len); > > + } > > } else { > > len = esp_fifo_pop_buf(s, buf, fifo8_num_used(&s->fifo)); > > len = MIN(fifo8_num_free(&s->cmdfifo), len); > } > fifo8_push_all(&s->cmdfifo, buf, len); > > Coverity reported access to uninitialized buf[]: > > >>> CID 1612373: Uninitialized variables (UNINIT) > >>> Using uninitialized value "*buf" when calling "fifo8_push_all". > > Do you mind having a look?
I think this is a false positive (and marked it that way in the Coverity Scan UI). Coverity is complaining that we might access buf[] uninitialized, but in the code path it is complaining about we know that len is zero. The fifo8_push_all() does a "memcpy(&fifo->data[start], data, num)" and if num is 0 that is valid and won't access anything in buf[]. We could I suppose make fifo8_push_all() return early for the num == 0 case, just to make it clearer that it's supposed to work. thanks -- PMM