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 <th...@redhat.com> >>> >>> 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 <th...@redhat.com> >>> Message-id: 1493883704-27604-1-git-send-email-th...@redhat.com >>> Signed-off-by: Gerd Hoffmann <kra...@redhat.com> >>> --- >>> 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... Paolo