Re: [Qemu-devel] [PATCH] mipsnet incorrect device ID fix

[Aurelien Jarno <aurelien@aurel32.net>]

On Fri, Feb 22, 2008 at 06:16:25AM +0530, Vijay Kumar wrote:
> The mipsnet device returns wrong values for device ID, since it returns  
> the contents of the pointer rather that the contents of the device ID  
> string. Also the contents of the string is returned such that the order  
> is host endianess dependent. The patch fixes both these issues.
>
> Signed-off-by: Vijay Kumar B. <vijaykumar@bravegnu.org>
>
> --- qemu-orig/hw/mipsnet.c	2007-12-27 11:18:52.000000000 +0530
> +++ qemu-mod/hw/mipsnet.c	2008-02-20 20:23:44.000000000 +0530
> @@ -101,6 +101,19 @@
>      mipsnet_update_irq(s);
>  }
>
> +static uint32_t bytes_to_int32(const unsigned char *arr)
> +{
> +    int i;
> +    uint32_t ret = 0;
> +    int nbytes = sizeof(int32_t);
> +
> +    for (i = 0; i < nbytes; i++) {
> +	ret = ret << 8 | arr[nbytes - 1 - i];
> +    }
> +
> +    return ret;
> +}

Why not use le32_to_cpu() which does the same, but in an optimized way?

>  static uint32_t mipsnet_ioport_read(void *opaque, uint32_t addr)
>  {
>      MIPSnetState *s = opaque;
> @@ -110,10 +123,10 @@
>      addr &= 0x3f;
>      switch (addr) {
>      case MIPSNET_DEV_ID:
> -	ret = *((uint32_t *)&devid);
> +	ret = bytes_to_int32(devid);

That's the '&' which is wrong here. The string can be accessed with
*((uint32_t *)devid). So you can simply use:

        ret = le32_to_cpu(*((uint32_t *)devid))

>          break;
>      case MIPSNET_DEV_ID + 4:
> -	ret = *((uint32_t *)(&devid + 4));
> +	ret = bytes_to_int32(devid + 4);

        ret = le32_to_cpu(*((uint32_t *)devid + 4));

>          break;
>      case MIPSNET_BUSY:
>  	ret = s->busy;
>
>
>

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net