[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation
From: |
Programmingkid |
Subject: |
Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation |
Date: |
Tue, 25 Aug 2015 10:33:17 -0400 |
On Aug 24, 2015, at 6:21 PM, Eric Blake wrote:
> 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...
I prefer to use well known functions that work well, but I guess it shouldn't
be too
painful to use the g_strdup_printf() function. Do you really think there is a
possible
overflow condition here?
>
>> + 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
Why do you believe this?
> we don't use \a BEL sequences anywhere else in qemu code.
Innovation has to start somewhere :)
>
>> + }
>> }
>>
>> if (dev->id) {
>
> This if would now be a dead check if your patch is applied.
I think you are right. It will be removed.
>
>> 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.
I didn't want to move the code. It just was in a condition that would never
be true, so I thought why keep it.
> --
> Eric Blake eblake redhat com +1-919-301-3266
Thank you very much for reviewing my patch.
> Libvirt virtualization library http://libvirt.org
You work with this project? Any chance libvirt could support Mac OS X?
- [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation, Programmingkid, 2015/08/24
- Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation, Eric Blake, 2015/08/24
- Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation, Markus Armbruster, 2015/08/25
- Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation, Programmingkid, 2015/08/25
- Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation, Peter Maydell, 2015/08/25
- Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation, Programmingkid, 2015/08/25
- Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation, Markus Armbruster, 2015/08/25
- Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation, Programmingkid, 2015/08/25
Re: [Qemu-devel] [PATCH] qdev-monitor.c: Add device id generation,
Programmingkid <=
[Qemu-devel] Should we auto-generate IDs? (was: [PATCH] qdev-monitor.c: Add device id generation), Markus Armbruster, 2015/08/25
- Re: [Qemu-devel] Should we auto-generate IDs? (was: [PATCH] qdev-monitor.c: Add device id generation), Programmingkid, 2015/08/25
- Re: [Qemu-devel] Should we auto-generate IDs? (was: [PATCH] qdev-monitor.c: Add device id generation), Programmingkid, 2015/08/26
- Re: [Qemu-devel] Should we auto-generate IDs?, Markus Armbruster, 2015/08/26
- Re: [Qemu-devel] Should we auto-generate IDs?, Programmingkid, 2015/08/26
- Re: [Qemu-devel] Should we auto-generate IDs?, Peter Maydell, 2015/08/26
- Re: [Qemu-devel] Should we auto-generate IDs?, Programmingkid, 2015/08/26
- Re: [Qemu-devel] Should we auto-generate IDs?, John Snow, 2015/08/26
- Re: [Qemu-devel] Should we auto-generate IDs?, Programmingkid, 2015/08/26
- Re: [Qemu-devel] Should we auto-generate IDs?, Markus Armbruster, 2015/08/27