On Tue, 17 Mar 2026 at 10:34, BALATON Zoltan <[email protected]> wrote: > > On Tue, 17 Mar 2026, Peter Maydell wrote: > > On Mon, 16 Mar 2026 at 00:06, BALATON Zoltan <[email protected]> wrote: > >> > >> Coverity reports possible sign extension error (CID 1645615) when > >> reading DEFAULT_SC_BOTTOM_RIGHT register. It cannot happen because the > >> values are limited when writing the register but change the code to > >> match other similar registers and to avoid this warning. > >> > >> Signed-off-by: BALATON Zoltan <[email protected]> > >> --- > >> hw/display/ati.c | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > >> > >> diff --git a/hw/display/ati.c b/hw/display/ati.c > >> index 1a6a5ad4fd..d194f15002 100644 > >> --- a/hw/display/ati.c > >> +++ b/hw/display/ati.c > >> @@ -513,8 +513,8 @@ static uint64_t ati_mm_read(void *opaque, hwaddr addr, > >> unsigned int size) > >> val |= s->regs.default_tile << 16; > >> break; > >> case DEFAULT_SC_BOTTOM_RIGHT: > >> - val = (s->regs.default_sc_bottom << 16) | > >> - s->regs.default_sc_right; > >> + val = s->regs.default_sc_right; > >> + val |= s->regs.default_sc_bottom << 16; > > > > I don't think this is going to make Coverity any happier, because > > we're still doing: > > * take a value in a 16-bit unsigned type > > * shift it without a cast, giving a 'signed int' (32-bit) > > * promote that to 64-bits to do the |=, because 'val' is > > 64 bits, causing a potential sign extension > > The rephrasing of the code doesn't avoid the sign-extension. > > OK then I'd additionally change val to uint32_t which should avoid it > then. Would that work?
Yes, I think that using uint32_t for val would silence the coverity warnings. thanks -- PMM
