| Message ID | 20200518173859.16520-2-f4bug@amsat.org |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] hw/m68k/mcf5206: Reduce m5206_mbar_read/write() offset arg to 16-bit | expand |
Am Mon, 18 May 2020 19:38:58 +0200 schrieb Philippe Mathieu-Daudé <f4bug@amsat.org>: > All calls to m5206_mbar_read/m5206_mbar_write are used with > 'offset = hwaddr & 0x3ff', so we are sure the offset fits > in 16-bit. > > Suggested-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/m68k/mcf5206.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c > index b155dd8170..45f44c43f0 100644 > --- a/hw/m68k/mcf5206.c > +++ b/hw/m68k/mcf5206.c > @@ -273,7 +273,7 @@ static void m5206_mbar_reset(m5206_mbar_state *s) > } > > static uint64_t m5206_mbar_read(m5206_mbar_state *s, > - uint64_t offset, unsigned size) > + uint16_t offset, unsigned size) > { > if (offset >= 0x100 && offset < 0x120) { > return m5206_timer_read(s->timer[0], offset - 0x100); > @@ -306,11 +306,11 @@ static uint64_t > m5206_mbar_read(m5206_mbar_state *s, case 0x170: return s->uivr[0]; > case 0x1b0: return s->uivr[1]; > } > - hw_error("Bad MBAR read offset 0x%x", (int)offset); > + hw_error("Bad MBAR read offset 0x%x", offset); > return 0; > } > > -static void m5206_mbar_write(m5206_mbar_state *s, uint32_t offset, > +static void m5206_mbar_write(m5206_mbar_state *s, uint16_t offset, > uint64_t value, unsigned size) > { > if (offset >= 0x100 && offset < 0x120) { > @@ -360,7 +360,7 @@ static void m5206_mbar_write(m5206_mbar_state *s, > uint32_t offset, s->uivr[1] = value; > break; > default: > - hw_error("Bad MBAR write offset 0x%x", (int)offset); > + hw_error("Bad MBAR write offset 0x%x", offset); Isn't the correct format string for short ints (i.e. 16-bit) rather %hx instead of %x ? ... I think it does not matter on x86, but IIRC there are other architectures where this could be a problem. I'd say, let's simply use "int" for offset instead, that's likely the best solution. (I can do the change when picking up the patch in case you don't want to respin, just let me know) Thomas
diff --git a/hw/m68k/mcf5206.c b/hw/m68k/mcf5206.c index b155dd8170..45f44c43f0 100644 --- a/hw/m68k/mcf5206.c +++ b/hw/m68k/mcf5206.c @@ -273,7 +273,7 @@ static void m5206_mbar_reset(m5206_mbar_state *s) } static uint64_t m5206_mbar_read(m5206_mbar_state *s, - uint64_t offset, unsigned size) + uint16_t offset, unsigned size) { if (offset >= 0x100 && offset < 0x120) { return m5206_timer_read(s->timer[0], offset - 0x100); @@ -306,11 +306,11 @@ static uint64_t m5206_mbar_read(m5206_mbar_state *s, case 0x170: return s->uivr[0]; case 0x1b0: return s->uivr[1]; } - hw_error("Bad MBAR read offset 0x%x", (int)offset); + hw_error("Bad MBAR read offset 0x%x", offset); return 0; } -static void m5206_mbar_write(m5206_mbar_state *s, uint32_t offset, +static void m5206_mbar_write(m5206_mbar_state *s, uint16_t offset, uint64_t value, unsigned size) { if (offset >= 0x100 && offset < 0x120) { @@ -360,7 +360,7 @@ static void m5206_mbar_write(m5206_mbar_state *s, uint32_t offset, s->uivr[1] = value; break; default: - hw_error("Bad MBAR write offset 0x%x", (int)offset); + hw_error("Bad MBAR write offset 0x%x", offset); break; } }
All calls to m5206_mbar_read/m5206_mbar_write are used with 'offset = hwaddr & 0x3ff', so we are sure the offset fits in 16-bit. Suggested-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/m68k/mcf5206.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)