| Message ID | 1492763327-12742-2-git-send-email-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
On 21 April 2017 at 09:28, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote: > This was an artifact from very early versions of the code from before the > memory API and is no longer needed. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/display/cg3.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/hw/display/cg3.c b/hw/display/cg3.c > index 1174220..7d43694 100644 > --- a/hw/display/cg3.c > +++ b/hw/display/cg3.c > @@ -26,7 +26,6 @@ > #include "qemu/osdep.h" > #include "qapi/error.h" > #include "qemu-common.h" > -#include "cpu.h" > #include "qemu/error-report.h" > #include "ui/console.h" > #include "hw/sysbus.h" > @@ -114,7 +113,7 @@ static void cg3_update_display(void *opaque) > for (y = 0; y < height; y++) { > int update = s->full_update; > > - page = (y * width) & TARGET_PAGE_MASK; > + page = y * width; Coverity warns that this is multiplying two 32 bit quantities and giving a 32 bit result that's then assigned to a 64 bit variable. Casting like: page = (ram_addr_t)y * width; is the usual way to placate it. thanks -- PMM
On 25/04/17 15:57, Peter Maydell wrote: > On 21 April 2017 at 09:28, Mark Cave-Ayland > <mark.cave-ayland@ilande.co.uk> wrote: >> This was an artifact from very early versions of the code from before the >> memory API and is no longer needed. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> >> --- >> hw/display/cg3.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/hw/display/cg3.c b/hw/display/cg3.c >> index 1174220..7d43694 100644 >> --- a/hw/display/cg3.c >> +++ b/hw/display/cg3.c >> @@ -26,7 +26,6 @@ >> #include "qemu/osdep.h" >> #include "qapi/error.h" >> #include "qemu-common.h" >> -#include "cpu.h" >> #include "qemu/error-report.h" >> #include "ui/console.h" >> #include "hw/sysbus.h" >> @@ -114,7 +113,7 @@ static void cg3_update_display(void *opaque) >> for (y = 0; y < height; y++) { >> int update = s->full_update; >> >> - page = (y * width) & TARGET_PAGE_MASK; >> + page = y * width; > > Coverity warns that this is multiplying two 32 bit quantities > and giving a 32 bit result that's then assigned to a 64 bit variable. > Casting like: > page = (ram_addr_t)y * width; > > is the usual way to placate it. Ah okay. I've done a quick test here and the above change seems fine so I'll send along a patch. ATB, Mark.
diff --git a/hw/display/cg3.c b/hw/display/cg3.c index 1174220..7d43694 100644 --- a/hw/display/cg3.c +++ b/hw/display/cg3.c @@ -26,7 +26,6 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "qemu-common.h" -#include "cpu.h" #include "qemu/error-report.h" #include "ui/console.h" #include "hw/sysbus.h" @@ -114,7 +113,7 @@ static void cg3_update_display(void *opaque) for (y = 0; y < height; y++) { int update = s->full_update; - page = (y * width) & TARGET_PAGE_MASK; + page = y * width; update |= memory_region_get_dirty(&s->vram_mem, page, page + width, DIRTY_MEMORY_VGA); if (update) { @@ -148,8 +147,7 @@ static void cg3_update_display(void *opaque) } if (page_max >= page_min) { memory_region_reset_dirty(&s->vram_mem, - page_min, page_max - page_min + TARGET_PAGE_SIZE, - DIRTY_MEMORY_VGA); + page_min, page_max - page_min, DIRTY_MEMORY_VGA); } /* vsync interrupt? */ if (s->regs[0] & CG3_CR_ENABLE_INTS) {