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.
Paolo
> @@ -554,10 +545,7 @@ static USBDevice *usb_serial_init(USBBus *bus, const
> char *filename)
>
> dev = usb_create(bus, "usb-serial");
> qdev_prop_set_chr(&dev->qdev, "chardev", cdrv);
> - if (vendorid)
> - qdev_prop_set_uint16(&dev->qdev, "vendorid", vendorid);
> - if (productid)
> - qdev_prop_set_uint16(&dev->qdev, "productid", productid);
> +
> return dev;
> }
>
>
