| Message ID | 1436815589-19194-1-git-send-email-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 13 July 2015 at 20:26, John Snow <jsnow@redhat.com> wrote: > Just a handful of casts to quiet coverity up. > > s->ports should never exceed 32, but coverity doesn't know that. > ncq_tfs->sector_count should also never exceed 64K. Personally I tend to mark that kind of thing as a false positive in the coverity UI and move on... -- PMM
On 07/13/2015 03:40 PM, Peter Maydell wrote: > On 13 July 2015 at 20:26, John Snow <jsnow@redhat.com> wrote: >> Just a handful of casts to quiet coverity up. >> >> s->ports should never exceed 32, but coverity doesn't know that. >> ncq_tfs->sector_count should also never exceed 64K. > > Personally I tend to mark that kind of thing as a false > positive in the coverity UI and move on... > > -- PMM > Either way; Paolo pinged me about the NCQ one so I figured I'd just do it. *shrug*
On 13/07/2015 21:41, John Snow wrote: >>> >> s->ports should never exceed 32, but coverity doesn't know that. >>> >> ncq_tfs->sector_count should also never exceed 64K. >> > >> > Personally I tend to mark that kind of thing as a false >> > positive in the coverity UI and move on... >> > >> > -- PMM >> > > Either way; Paolo pinged me about the NCQ one so I figured I'd just do it. Yeah, neither is particularly optimal. Every now and then (a couple years, say) you do have to re-evaluate false positives, so it's better to fix them if possible. On the other hand the code is uglier. Let's ignore these in Coverity---with a triaging comment there about why they are false positives. Paolo
On 07/14/2015 04:30 AM, Paolo Bonzini wrote: > > > On 13/07/2015 21:41, John Snow wrote: >>>>>> s->ports should never exceed 32, but coverity doesn't know that. >>>>>> ncq_tfs->sector_count should also never exceed 64K. >>>> >>>> Personally I tend to mark that kind of thing as a false >>>> positive in the coverity UI and move on... >>>> >>>> -- PMM >>>> >> Either way; Paolo pinged me about the NCQ one so I figured I'd just do it. > > Yeah, neither is particularly optimal. Every now and then (a couple > years, say) you do have to re-evaluate false positives, so it's better > to fix them if possible. On the other hand the code is uglier. > > Let's ignore these in Coverity---with a triaging comment there about why > they are false positives. > > Paolo > Alright, I'll follow your lead on this and just adjust the Coverity triaging comments. --js
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index bb6a92f..c45983c 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -356,7 +356,7 @@ static uint64_t ahci_mem_read_32(void *opaque, hwaddr addr) DPRINTF(-1, "(addr 0x%08X), val 0x%08X\n", (unsigned) addr, val); } else if ((addr >= AHCI_PORT_REGS_START_ADDR) && (addr < (AHCI_PORT_REGS_START_ADDR + - (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) { + (s->ports * (hwaddr)AHCI_PORT_ADDR_OFFSET_LEN)))) { val = ahci_port_read(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7, addr & AHCI_PORT_ADDR_OFFSET_MASK); } @@ -433,7 +433,7 @@ static void ahci_mem_write(void *opaque, hwaddr addr, } } else if ((addr >= AHCI_PORT_REGS_START_ADDR) && (addr < (AHCI_PORT_REGS_START_ADDR + - (s->ports * AHCI_PORT_ADDR_OFFSET_LEN)))) { + (s->ports * (hwaddr)AHCI_PORT_ADDR_OFFSET_LEN)))) { ahci_port_write(s, (addr - AHCI_PORT_REGS_START_ADDR) >> 7, addr & AHCI_PORT_ADDR_OFFSET_MASK, val); } @@ -1571,8 +1571,8 @@ static int ahci_state_post_load(void *opaque, int version_id) return -1; } ahci_populate_sglist(ncq_tfs->drive, &ncq_tfs->sglist, - ncq_tfs->cmdh, ncq_tfs->sector_count * 512, - 0); + ncq_tfs->cmdh, + (int64_t)ncq_tfs->sector_count * 512, 0); if (ncq_tfs->sector_count != ncq_tfs->sglist.size >> 9) { return -1; } diff --git a/hw/ide/macio.c b/hw/ide/macio.c index a55a479..340774b 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -253,7 +253,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) } /* Calculate current offset */ - offset = (int64_t)(s->lba << 11) + s->io_buffer_index; + offset = ((int64_t)s->lba << 11) + s->io_buffer_index; pmac_dma_read(s->blk, offset, io->len, pmac_ide_atapi_transfer_cb, io); return;
Just a handful of casts to quiet coverity up. s->ports should never exceed 32, but coverity doesn't know that. ncq_tfs->sector_count should also never exceed 64K. Why not make Coverity a happy camper, though. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/ide/ahci.c | 8 ++++---- hw/ide/macio.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)