[Qemu-devel] Re: [PATCH 03/20] pci: simplify pci_data_read(), pcie_mmcfg_data_read().

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Nov 12, 2009 at 02:58:31PM +0900, Isaku Yamahata wrote:
> simplify ugly switch by memcpy trick.
> And add one assert().
> 
> Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>

In fact, there's no reason to be so careful about
zeroing out high bits as far as I can tell:
in the end, the value is assigned to uint32/uint16/uint8
as appropriate and high bits are truncated.
So this can simply be val = 0xffffffff;

> ---
>  hw/pci_host.c  |   16 ++++------------
>  hw/pcie_host.c |   16 ++++------------
>  2 files changed, 8 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/pci_host.c b/hw/pci_host.c
> index f4518dc..4196ebc 100644
> --- a/hw/pci_host.c
> +++ b/hw/pci_host.c
> @@ -71,19 +71,11 @@ uint32_t pci_data_read(PCIBus *s, uint32_t addr, int len)
>      uint32_t config_addr = pci_addr_to_config(addr);
>      uint32_t val;
>  
> +    assert(len == 1 || len == 2 || len == 4);
>      if (!pci_dev) {
> -        switch(len) {
> -        case 1:
> -            val = 0xff;
> -            break;
> -        case 2:
> -            val = 0xffff;
> -            break;
> -        default:
> -        case 4:
> -            val = 0xffffffff;
> -            break;
> -        }
> +        val = 0;
> +        memset(&val, 0xff, len);
> +        val = le32_to_cpu(val);


So the above will become simply
if (!pci_dev) {
	return ~0x0;
}


>      } else {
>          val = pci_dev->config_read(pci_dev, config_addr, len);
>          PCI_DPRINTF("%s: %s: addr=%02"PRIx32" val=%08"PRIx32" len=%d\n",
> diff --git a/hw/pcie_host.c b/hw/pcie_host.c
> index b52fec6..61285da 100644
> --- a/hw/pcie_host.c
> +++ b/hw/pcie_host.c
> @@ -71,19 +71,11 @@ static uint32_t pcie_mmcfg_data_read(PCIBus *s,
>      PCIDevice *pci_dev = pcie_mmcfg_addr_to_dev(s, mmcfg_addr);
>      uint32_t val;
>  
> +    assert(len == 1 || len == 2 || len == 4);
>      if (!pci_dev) {
> -        switch(len) {
> -        case 1:
> -            val = 0xff;
> -            break;
> -        case 2:
> -            val = 0xffff;
> -            break;
> -        default:
> -        case 4:
> -            val = 0xffffffff;
> -            break;
> -        }
> +        val = 0;
> +        memset(&val, 0xff, len);
> +        val = le32_to_cpu(val);
>      } else {
>          val = pci_dev->config_read(pci_dev,
>                                     PCIE_MMCFG_CONFOFFSET(mmcfg_addr), len);
> -- 
> 1.6.0.2