Alex reported an uninitialized variable warning in ahci_populate_sglist.
Even though the warning is bogus and happens only because of -Og, the
code in the function leaves something to be desired; the condition that
triggers the warning is easily shown to be entirely redundant.
In particular, the loop's "if" condition can be rewritten from
"offset < sum + tbl_entry_size" to "offset - sum < tbl_entry_size";
this is safe since the LHS cannot underflow. Because off_pos is
exactly "offset - sum" it is clear that it can never be less than
zero or greater than tbl_entry_size. We can therefore keep the off_idx
check only and, for documentation purposes, reduce off_pos to an unsigned
32-bit integer.
The tracepoint also is not particularly useful at this point, since
we know that (if it ever triggers) off_idx will be -1 and off_pos
uninitialized. Instead, include the requested offset and the total PRDT
length, which will be smaller than the offset.
Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
---
hw/ide/ahci.c | 12 +++++-------
hw/ide/trace-events | 2 +-
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 680304a24c..997b67a6fc 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -924,8 +924,7 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList
*sglist,
int r = 0;
uint64_t sum = 0;
int off_idx = -1;
- int64_t off_pos = -1;
- int tbl_entry_size;
+ uint32_t off_pos = 0;
IDEBus *bus = &ad->port;
BusState *qbus = BUS(bus);
@@ -952,19 +951,18 @@ static int ahci_populate_sglist(AHCIDevice *ad,
QEMUSGList *sglist,
/* Get entries in the PRDT, init a qemu sglist accordingly */
if (prdtl > 0) {
AHCI_SG *tbl = (AHCI_SG *)prdt;
- sum = 0;
for (i = 0; i < prdtl; i++) {
- tbl_entry_size = prdt_tbl_entry_size(&tbl[i]);
- if (offset < (sum + tbl_entry_size)) {
+ uint32_t tbl_entry_size = prdt_tbl_entry_size(&tbl[i]);
+ if (offset - sum < tbl_entry_size) {
off_idx = i;
off_pos = offset - sum;
break;
}
sum += tbl_entry_size;
}
- if ((off_idx == -1) || (off_pos < 0) || (off_pos > tbl_entry_size)) {
+ if (off_idx == -1) {
trace_ahci_populate_sglist_bad_offset(ad->hba, ad->port_no,
- off_idx, off_pos);
+ sum, offset);
r = -1;
goto out;
}
diff --git a/hw/ide/trace-events b/hw/ide/trace-events
index 6e357685f9..81706efe80 100644
--- a/hw/ide/trace-events
+++ b/hw/ide/trace-events
@@ -88,7 +88,7 @@ ahci_populate_sglist(void *s, int port) "ahci(%p)[%d]"
ahci_populate_sglist_no_prdtl(void *s, int port, uint16_t opts) "ahci(%p)[%d]:
no sg list given by guest: 0x%04x"
ahci_populate_sglist_no_map(void *s, int port) "ahci(%p)[%d]: DMA mapping
failed"
ahci_populate_sglist_short_map(void *s, int port) "ahci(%p)[%d]: mapped less
than expected"
-ahci_populate_sglist_bad_offset(void *s, int port, int off_idx, int64_t
off_pos) "ahci(%p)[%d]: Incorrect offset! off_idx: %d, off_pos: %"PRId64
+ahci_populate_sglist_bad_offset(void *s, int port, uint64_t sum, uint64_t
offset) "ahci(%p)[%d]: Incorrect offset! total PRDT length %"PRIu64", offset:
%"PRIu64
ncq_finish(void *s, int port, uint8_t tag) "ahci(%p)[%d][tag:%d]: NCQ transfer
finished"
execute_ncq_command_read(void *s, int port, uint8_t tag, int count, int64_t
lba) "ahci(%p)[%d][tag:%d]: NCQ reading %d sectors from LBA %"PRId64
execute_ncq_command_unsup(void *s, int port, uint8_t tag, uint8_t cmd)
"ahci(%p)[%d][tag:%d]: error: unsupported NCQ command (0x%02x) received"
--
2.26.2
