[PATCH] ide: clean up ahci_populate_sglist

[Paolo Bonzini]

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 <pbonzini@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