[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendori
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendorid or productid properties |
Date: |
Thu, 18 May 2017 16:05:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 |
On 18.05.2017 15:35, Paolo Bonzini wrote:
>
>
> On 18/05/2017 15:22, Thomas Huth wrote:
>> On 18.05.2017 14:00, Paolo Bonzini wrote:
>>>
>>>
>>> On 12/05/2017 14:21, Gerd Hoffmann wrote:
>>>> From: Thomas Huth <address@hidden>
>>>>
>>>> When starting QEMU with the legacy USB serial device like this:
>>>>
>>>> qemu-system-x86_64 -usbdevice serial:vendorid=0x1234:stdio
>>>>
>>>> it currently aborts since the vendorid property does not exist
>>>> anymore (it has been removed by commit f29783f72ea77dfbd7ea0c9):
>>>>
>>>> Unexpected error in object_property_find() at qemu/qom/object.c:1008:
>>>> qemu-system-x86_64: -usbdevice serial:vendorid=0x1234:stdio: Property
>>>> '.vendorid' not found
>>>> Aborted (core dumped)
>>>>
>>>> Fix this crash by issuing a more friendly error message instead
>>>> (and simplify the code also a little bit this way).
>>>>
>>>> Signed-off-by: Thomas Huth <address@hidden>
>>>> Message-id: address@hidden
>>>> Signed-off-by: Gerd Hoffmann <address@hidden>
>>>> ---
>>>> hw/usb/dev-serial.c | 24 ++++++------------------
>>>> 1 file changed, 6 insertions(+), 18 deletions(-)
>>>>
>>>> diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
>>>> index 6d5137383b..83a4f0e6fb 100644
>>>> --- a/hw/usb/dev-serial.c
>>>> +++ b/hw/usb/dev-serial.c
>>>> @@ -513,27 +513,18 @@ static USBDevice *usb_serial_init(USBBus *bus, const
>>>> char *filename)
>>>> {
>>>> USBDevice *dev;
>>>> Chardev *cdrv;
>>>> - uint32_t vendorid = 0, productid = 0;
>>>> char label[32];
>>>> static int index;
>>>>
>>>> while (*filename && *filename != ':') {
>>>> const char *p;
>>>> - char *e;
>>>> +
>>>> if (strstart(filename, "vendorid=", &p)) {
>>>> - vendorid = strtol(p, &e, 16);
>>>> - if (e == p || (*e && *e != ',' && *e != ':')) {
>>>> - error_report("bogus vendor ID %s", p);
>>>> - return NULL;
>>>> - }
>>>> - filename = e;
>>>> + error_report("vendorid is not supported anymore");
>>>> + return NULL;
>>>> } else if (strstart(filename, "productid=", &p)) {
>>>> - productid = strtol(p, &e, 16);
>>>> - if (e == p || (*e && *e != ',' && *e != ':')) {
>>>> - error_report("bogus product ID %s", p);
>>>> - return NULL;
>>>> - }
>>>> - filename = e;
>>>> + error_report("productid is not supported anymore");
>>>> + return NULL;
>>>> } else {
>>>> error_report("unrecognized serial USB option %s", filename);
>>>> return NULL;
>>>
>>> All breanches of the "if" now return NULL, so the "while" loop in turn
>>> can become an
>>>
>>> if (*filename && *filename != ':') {
>>> }
>>>
>>> and the "while (*filename == ',')" subloop can go away, replaced by just
>>> "return NULL".
>>>
>>> Even better, the "if (!*filename)" if just below can be moved first.
>>
>> Feel free to send an additional cleanup patch ... otherwise, I'd say let
>> it bitrot for another year and we then remove it completely together
>> with all the other "-usbdevice" functions...
>
> Well, Coverity reports it so I'd rather keep it clean...
Hmm, maybe we should simply remove "-usbdevice serial" right now already
... ? The vendorid/productid parameter handling has been broken since
QEMU v0.14 already and nobody ever complained, so I guess hardly anybody
is using "-usbdevice serial" anymore ... so I tend to simply remove it
directly instead of going through the typical "mark-as-deprecated ->
wait-two-release-cycles -> finally-remove-it" process here...
Paolo, Gerd, what do you think?
Thomas
- [Qemu-devel] [PULL 0/6] usb patch queue, Gerd Hoffmann, 2017/05/12
- [Qemu-devel] [PULL 2/6] usb-redir: fix stack overflow in usbredir_log_data, Gerd Hoffmann, 2017/05/12
- [Qemu-devel] [PULL 3/6] xhci: fix logging, Gerd Hoffmann, 2017/05/12
- [Qemu-devel] [PULL 4/6] usb-hub: clear PORT_STAT_SUSPEND on wakeup, Gerd Hoffmann, 2017/05/12
- [Qemu-devel] [PULL 1/6] qemu-doc: Update to use the new way of attaching USB devices, Gerd Hoffmann, 2017/05/12
- [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendorid or productid properties, Gerd Hoffmann, 2017/05/12
- Re: [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendorid or productid properties, Paolo Bonzini, 2017/05/18
- Re: [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendorid or productid properties, Thomas Huth, 2017/05/18
- Re: [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendorid or productid properties, Paolo Bonzini, 2017/05/18
- Re: [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendorid or productid properties,
Thomas Huth <=
- Re: [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendorid or productid properties, Gerd Hoffmann, 2017/05/18
- Re: [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendorid or productid properties, Markus Armbruster, 2017/05/19
- Re: [Qemu-devel] [PULL 6/6] hw/usb/dev-serial: Do not try to set vendorid or productid properties, Thomas Huth, 2017/05/19
[Qemu-devel] [PULL 5/6] xhci: relax link check, Gerd Hoffmann, 2017/05/12
Re: [Qemu-devel] [PULL 0/6] usb patch queue, Stefan Hajnoczi, 2017/05/15