Re: [Qemu-devel] [PATCH 2/9] Change signature of address_space_read() to avoid casting

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6678 bytes --]

On Thu, Jul 07, 2016 at 04:33:36PM -0700, Andrey Smirnov wrote:
> Change signature of address_space_read() to expectet void * as a buffer
> instead of uint8_t * to avoid forcing the caller of the function to do a
> type cast.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

Seems reasonable.

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  exec.c                |  2 +-
>  hw/net/dp8393x.c      | 16 ++++++++--------
>  hw/virtio/virtio.c    |  2 +-
>  include/exec/memory.h |  2 +-
>  4 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 93611f1..1bd2204 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2736,7 +2736,7 @@ MemTxResult address_space_rw(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
>      if (is_write) {
>          return address_space_write(as, addr, attrs, (uint8_t *)buf, len);
>      } else {
> -        return address_space_read(as, addr, attrs, (uint8_t *)buf, len);
> +        return address_space_read(as, addr, attrs, buf, len);
>      }
>  }
>  
> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c
> index 2849542..06dc99b 100644
> --- a/hw/net/dp8393x.c
> +++ b/hw/net/dp8393x.c
> @@ -205,7 +205,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
>          /* Fill current entry */
>          address_space_read(&s->as,
>              (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
> -            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
> +            MEMTXATTRS_UNSPECIFIED, data, size);
>          s->cam[index][0] = data[1 * width] & 0xff;
>          s->cam[index][1] = data[1 * width] >> 8;
>          s->cam[index][2] = data[2 * width] & 0xff;
> @@ -224,7 +224,7 @@ static void dp8393x_do_load_cam(dp8393xState *s)
>      /* Read CAM enable */
>      address_space_read(&s->as,
>          (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP],
> -        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
> +        MEMTXATTRS_UNSPECIFIED, data, size);
>      s->regs[SONIC_CE] = data[0 * width];
>      DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]);
>  
> @@ -244,7 +244,7 @@ static void dp8393x_do_read_rra(dp8393xState *s)
>      size = sizeof(uint16_t) * 4 * width;
>      address_space_read(&s->as,
>          (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP],
> -        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
> +        MEMTXATTRS_UNSPECIFIED, data, size);
>  
>      /* Update SONIC registers */
>      s->regs[SONIC_CRBA0] = data[0 * width];
> @@ -362,7 +362,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
>          s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA];
>          address_space_read(&s->as,
>              ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * width,
> -            MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
> +            MEMTXATTRS_UNSPECIFIED, data, size);
>          tx_len = 0;
>  
>          /* Update registers */
> @@ -397,7 +397,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
>                  size = sizeof(uint16_t) * 3 * width;
>                  address_space_read(&s->as,
>                      ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * i) * width,
> -                    MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
> +                    MEMTXATTRS_UNSPECIFIED, data, size);
>                  s->regs[SONIC_TSA0] = data[0 * width];
>                  s->regs[SONIC_TSA1] = data[1 * width];
>                  s->regs[SONIC_TFS] = data[2 * width];
> @@ -438,7 +438,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s)
>              size = sizeof(uint16_t) * width;
>              address_space_read(&s->as,
>                  ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) * width,
> -                MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
> +                MEMTXATTRS_UNSPECIFIED, data, size);
>              s->regs[SONIC_CTDA] = data[0 * width] & ~0x1;
>              if (data[0 * width] & 0x1) {
>                  /* EOL detected */
> @@ -702,7 +702,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
>          size = sizeof(uint16_t) * 1 * width;
>          address = ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width;
>          address_space_read(&s->as, address, MEMTXATTRS_UNSPECIFIED,
> -                           (uint8_t *)data, size);
> +                           data, size);
>          if (data[0 * width] & 0x1) {
>              /* Still EOL ; stop reception */
>              return -1;
> @@ -760,7 +760,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf,
>      size = sizeof(uint16_t) * width;
>      address_space_read(&s->as,
>          ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width,
> -        MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size);
> +        MEMTXATTRS_UNSPECIFIED, data, size);
>      s->regs[SONIC_LLFA] = data[0 * width];
>      if (s->regs[SONIC_LLFA] & 0x1) {
>          /* EOL detected */
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 7ed06ea..449f125 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -122,7 +122,7 @@ static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
>                              hwaddr desc_pa, int i)
>  {
>      address_space_read(&address_space_memory, desc_pa + i * sizeof(VRingDesc),
> -                       MEMTXATTRS_UNSPECIFIED, (void *)desc, sizeof(VRingDesc));
> +                       MEMTXATTRS_UNSPECIFIED, desc, sizeof(VRingDesc));
>      virtio_tswap64s(vdev, &desc->addr);
>      virtio_tswap32s(vdev, &desc->len);
>      virtio_tswap16s(vdev, &desc->flags);
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 4ab6800..576c5a5 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -1418,7 +1418,7 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write)
>   */
>  static inline __attribute__((__always_inline__))
>  MemTxResult address_space_read(AddressSpace *as, hwaddr addr, MemTxAttrs attrs,
> -                               uint8_t *buf, int len)
> +                               void *buf, int len)
>  {
>      MemTxResult result = MEMTX_OK;
>      hwaddr l, addr1;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]