Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation

[Eric Blake]

On 08/24/2015 12:53 PM, Programmingkid wrote:
> Add device ID generation to each device if an ID isn't given.
> 
> Signed-off-by: John Arbuckle <address@hidden>
> 
> ---

>          dev->id = id;
> +    } else { /* create an id for a device if none is provided */
> +        static int device_id_count;
> +
> +        /* Add one for '\0' character */
> +        char *device_id = (char *) malloc(sizeof(char) *
> +                                            MAX_NUM_DIGITS_FOR_USB_ID + 1);
> +        sprintf(device_id, "%d", device_id_count++);

g_strdup_printf() is a lot nicer about avoiding the risk of arbitrary
overflow...

> +        dev->id = (const char *) device_id;
> +
> +        /* if device_id_count >= 10^MAX_NUM_DIGITS_FOR_USB_ID */
> +        if (device_id_count >= pow(10, MAX_NUM_DIGITS_FOR_USB_ID)) {
> +            printf("Warning: Maximum number of device ID's generated!\n\a");
> +            printf("Time for you to make your own device ID's.\n");

besides, printf() is probably the wrong way to do error reporting, and
we don't use \a BEL sequences anywhere else in qemu code.

> +        }
>      }
>  
>      if (dev->id) {

This if would now be a dead check if your patch is applied.

>          object_property_add_child(qdev_get_peripheral(), dev->id,
>                                    OBJECT(dev), NULL);
> -    } else {
> -        static int anon_count;
> -        gchar *name = g_strdup_printf("device[%d]", anon_count++);
> -        object_property_add_child(qdev_get_peripheral_anon(), name,
> -                                  OBJECT(dev), NULL);
> -        g_free(name);
>      }

It looks like your goal was to move this code earlier, but you changed
enough aspects of it that I'm not sure what the right fix should be.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature