Re: [Qemu-devel] [PATCH 2/2] lsi: use ldn_le_p()/stn_le_p()

[Peter Maydell]

On Mon, 18 Feb 2019 at 21:41, Philippe Mathieu-Daudé <address@hidden> wrote:
>
> Hi Sven,
>
> On 2/18/19 6:55 PM, Sven Schnelle wrote:
> > Signed-off-by: Sven Schnelle <address@hidden>
> > ---
> >  hw/scsi/lsi53c895a.c | 19 ++-----------------
> >  1 file changed, 2 insertions(+), 17 deletions(-)
> >
> > diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> > index c493e3c4c7..93c4434bfb 100644
> > --- a/hw/scsi/lsi53c895a.c
> > +++ b/hw/scsi/lsi53c895a.c
> > @@ -2078,29 +2078,14 @@ static void lsi_ram_write(void *opaque, hwaddr addr,
> >                            uint64_t val, unsigned size)
> >  {
> >      LSIState *s = opaque;
> > -    uint32_t newval;
> > -    uint32_t mask;
> > -    int shift;
> > -
> > -    newval = s->script_ram[addr >> 2];
> > -    shift = (addr & 3) * 8;
> > -    mask = ((uint64_t)1 << (size * 8)) - 1;
> > -    newval &= ~(mask << shift);
> > -    newval |= val << shift;
> > -    s->script_ram[addr >> 2] = newval;
> > +    stn_le_p(((void*)s->script_ram) + addr, size, val);
>
> If you want to do pointer arithmetic, it is safer to cast to a uintptr_t.
> But since you update all the places that use script_ram[], it seems
> pointless to keep it as an array of uint32_t. We can simply convert it
> to an array of char.

Ah, yes -- when I suggested the cast on #qemu I hadn't
realized that these read and write functions were the
only places that access script_ram[] -- I'd assumed
that some code in the device model was going to read it to
execute whatever these scripts are.

thanks
-- PMM