On 31/7/24 17:13, Kevin Wolf wrote:
> Am 31.07.2024 um 16:36 hat Peter Maydell geschrieben:
>> Coverity notes that the code at the end of the loop in
>> bmdma_prepare_buf() is unreachable. This is because in commit
>> 9fbf0fa81fca8f527 ("ide: remove hardcoded 2GiB transactional limit")
>> we removed the only codepath in the loop which could "break" out of
>> it, but didn't notice that this meant we should also remove the code
>> at the end of the loop.
>>
>> Remove the dead code.
>>
>> Resolves: Coverity CID 1547772
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> hw/ide/pci.c | 4 ----
>> 1 file changed, 4 deletions(-)
>>
>> diff --git a/hw/ide/pci.c b/hw/ide/pci.c
>> index 4675d079a17..f2cb500a94f 100644
>> --- a/hw/ide/pci.c
>> +++ b/hw/ide/pci.c
>> @@ -266,10 +266,6 @@ static int32_t bmdma_prepare_buf(const IDEDMA *dma, int32_t limit)
>> s->io_buffer_size += l;
>> }
>> }
>> -
>> - qemu_sglist_destroy(&s->sg);
>> - s->io_buffer_size = 0;
>> - return -1;
>> }
>
> Should we put a g_assert_not_reached() here instead to make it easier
> for the reader to understand how this function works?
Or break and keep returning at EOF:
-- >8 --
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 4675d079a1..8386c4fe42 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -237,7 +237,7 @@ static int32_t bmdma_prepare_buf(const IDEDMA *dma,
int32_t limit)
/* end of table (with a fail safe of one page) */
if (bm->cur_prd_last ||
(bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE) {
- return s->sg.size;
+ break;
}
pci_dma_read(pci_dev, bm->cur_addr, &prd, 8);
bm->cur_addr += 8;
@@ -267,9 +267,7 @@ static int32_t bmdma_prepare_buf(const IDEDMA *dma,
int32_t limit)
}
}
- qemu_sglist_destroy(&s->sg);
- s->io_buffer_size = 0;
- return -1;
+ return s->sg.size;
}
---
>
> Either way:
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>