Re: [Qemu-devel] [PATCH 1/3] libqos: Remove PCI assumptions in virtio driver

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Thu, Sep 04, 2014 at 06:24:37PM +0200, Marc Marí wrote:
> @@ -60,25 +60,25 @@ static void qvirtio_pci_assign_device(QVirtioDevice *d, void *data)
>      *vpcidev = (QVirtioPCIDevice *)d;
>  }
>  
> -static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, void *addr)
> +static uint8_t qvirtio_pci_config_readb(QVirtioDevice *d, uint64_t addr)
>  {
>      QVirtioPCIDevice *dev = (QVirtioPCIDevice *)d;
> -    return qpci_io_readb(dev->pdev, addr);
> +    return qpci_io_readb(dev->pdev, (void *)addr);

You do not need casts in C for void* to any pointer type or any pointer
type to void*.  Please drop them.

> diff --git a/tests/libqos/virtio-pci.h b/tests/libqos/virtio-pci.h
> index 883f7ff..8f0e52a 100644
> --- a/tests/libqos/virtio-pci.h
> +++ b/tests/libqos/virtio-pci.h
> @@ -13,18 +13,18 @@
>  #include "libqos/virtio.h"
>  #include "libqos/pci.h"
>  
> -#define QVIRTIO_DEVICE_FEATURES         0x00
> -#define QVIRTIO_GUEST_FEATURES          0x04
> -#define QVIRTIO_QUEUE_ADDRESS           0x08
> -#define QVIRTIO_QUEUE_SIZE              0x0C
> -#define QVIRTIO_QUEUE_SELECT            0x0E
> -#define QVIRTIO_QUEUE_NOTIFY            0x10
> -#define QVIRTIO_DEVICE_STATUS           0x12
> -#define QVIRTIO_ISR_STATUS              0x13
> -#define QVIRTIO_MSIX_CONF_VECTOR        0x14
> -#define QVIRTIO_MSIX_QUEUE_VECTOR       0x16
> -#define QVIRTIO_DEVICE_SPECIFIC_MSIX    0x18
> -#define QVIRTIO_DEVICE_SPECIFIC_NO_MSIX 0x14
> +#define QVIRTIO_PCI_DEVICE_FEATURES         0x00
> +#define QVIRTIO_PCI_GUEST_FEATURES          0x04
> +#define QVIRTIO_PCI_QUEUE_ADDRESS           0x08
> +#define QVIRTIO_PCI_QUEUE_SIZE              0x0C
> +#define QVIRTIO_PCI_QUEUE_SELECT            0x0E
> +#define QVIRTIO_PCI_QUEUE_NOTIFY            0x10
> +#define QVIRTIO_PCI_DEVICE_STATUS           0x12
> +#define QVIRTIO_PCI_ISR_STATUS              0x13
> +#define QVIRTIO_PCI_MSIX_CONF_VECTOR        0x14
> +#define QVIRTIO_PCI_MSIX_QUEUE_VECTOR       0x16
> +#define QVIRTIO_PCI_DEVICE_SPECIFIC_MSIX    0x18
> +#define QVIRTIO_PCI_DEVICE_SPECIFIC_NO_MSIX 0x14

This is a nice isolated change that could be made in a separate commit
from the void* -> uint64_t change.

It's a matter of style but makes code review easier since it is harder
to review a patch that is performing multiple changes at once.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]