| Message ID | 57FA8C0D-64FF-4F71-8597-E36E0D1FF7E4@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Mo, 2015-08-24 at 15:13 -0400, Programmingkid wrote: > + monitor_printf(mon, " Device %d.%d, Port %s, Speed %s > Mb/s," > + "Product %s, ID %s\n", > + bus->busnr, dev->addr, port->path, > + usb_speed(dev->speed), > + dev->product_desc, dev->qdev.id); > dev->qdev.id can be NULL. cheers, Gerd
On Aug 28, 2015, at 7:50 AM, Gerd Hoffmann wrote: > On Mo, 2015-08-24 at 15:13 -0400, Programmingkid wrote: >> + monitor_printf(mon, " Device %d.%d, Port %s, Speed %s >> Mb/s," >> + "Product %s, ID %s\n", >> + bus->busnr, dev->addr, port->path, >> + usb_speed(dev->speed), >> + dev->product_desc, dev->qdev.id); >> > > dev->qdev.id can be NULL. > > cheers, > Gerd That isn't a problem. It will just say "null" when it is printed. No crash or garbage text.
On Fri, Aug 28, 2015 at 09:08:12AM -0400, Programmingkid wrote: > > On Aug 28, 2015, at 7:50 AM, Gerd Hoffmann wrote: > > > On Mo, 2015-08-24 at 15:13 -0400, Programmingkid wrote: > >> + monitor_printf(mon, " Device %d.%d, Port %s, Speed %s > >> Mb/s," > >> + "Product %s, ID %s\n", > >> + bus->busnr, dev->addr, port->path, > >> + usb_speed(dev->speed), > >> + dev->product_desc, dev->qdev.id); > >> > > > > dev->qdev.id can be NULL. > > > > cheers, > > Gerd > > That isn't a problem. It will just say "null" when it is printed. > No crash or garbage text. Handling of '%s' with NULL is undefined by the standard. Linux glibc prints the string "null", but other implementations can crash. Regards, Daniel
On Aug 28, 2015, at 9:35 AM, Daniel P. Berrange wrote: > On Fri, Aug 28, 2015 at 09:08:12AM -0400, Programmingkid wrote: >> >> On Aug 28, 2015, at 7:50 AM, Gerd Hoffmann wrote: >> >>> On Mo, 2015-08-24 at 15:13 -0400, Programmingkid wrote: >>>> + monitor_printf(mon, " Device %d.%d, Port %s, Speed %s >>>> Mb/s," >>>> + "Product %s, ID %s\n", >>>> + bus->busnr, dev->addr, port->path, >>>> + usb_speed(dev->speed), >>>> + dev->product_desc, dev->qdev.id); >>>> >>> >>> dev->qdev.id can be NULL. >>> >>> cheers, >>> Gerd >> >> That isn't a problem. It will just say "null" when it is printed. >> No crash or garbage text. > > Handling of '%s' with NULL is undefined by the standard. Linux > glibc prints the string "null", but other implementations can > crash. "null" is also printed on Mac OS X. Do you want dev->qdev.id replaced with this? (dev->qdev.id != NULL) ? dev->qdev.id : "null"
On Fri, Aug 28, 2015 at 09:39:43AM -0400, Programmingkid wrote: > > On Aug 28, 2015, at 9:35 AM, Daniel P. Berrange wrote: > > > On Fri, Aug 28, 2015 at 09:08:12AM -0400, Programmingkid wrote: > >> > >> On Aug 28, 2015, at 7:50 AM, Gerd Hoffmann wrote: > >> > >>> On Mo, 2015-08-24 at 15:13 -0400, Programmingkid wrote: > >>>> + monitor_printf(mon, " Device %d.%d, Port %s, Speed %s > >>>> Mb/s," > >>>> + "Product %s, ID %s\n", > >>>> + bus->busnr, dev->addr, port->path, > >>>> + usb_speed(dev->speed), > >>>> + dev->product_desc, dev->qdev.id); > >>>> > >>> > >>> dev->qdev.id can be NULL. > >>> > >>> cheers, > >>> Gerd > >> > >> That isn't a problem. It will just say "null" when it is printed. > >> No crash or garbage text. > > > > Handling of '%s' with NULL is undefined by the standard. Linux > > glibc prints the string "null", but other implementations can > > crash. > > "null" is also printed on Mac OS X. I don't know if it is still true in latest versions, but historically Solaris would crash on NULL. "The problem is that while you can printf("%s",NULL); in most Linux distributions, doing the same in Solaris caused the executable to exit and generate a core." https://blogs.oracle.com/bnitz/entry/asc_and_sprintf_s_null > Do you want dev->qdev.id replaced with this? (dev->qdev.id != NULL) ? dev->qdev.id : "null" It can be a bit shorter if you just leave out the "!= NULL" bit and rely on implicit conversion of non-null to a boolean true. I'm not sure if QEMU has a macro wrapper for hiding this possibly. Regards, Daniel
On Fr, 2015-08-28 at 09:39 -0400, Programmingkid wrote: > On Aug 28, 2015, at 9:35 AM, Daniel P. Berrange wrote: > > > On Fri, Aug 28, 2015 at 09:08:12AM -0400, Programmingkid wrote: > >> > >> On Aug 28, 2015, at 7:50 AM, Gerd Hoffmann wrote: > >> > >>> On Mo, 2015-08-24 at 15:13 -0400, Programmingkid wrote: > >>>> + monitor_printf(mon, " Device %d.%d, Port %s, Speed %s > >>>> Mb/s," > >>>> + "Product %s, ID %s\n", > >>>> + bus->busnr, dev->addr, port->path, > >>>> + usb_speed(dev->speed), > >>>> + dev->product_desc, dev->qdev.id); > >>>> > >>> > >>> dev->qdev.id can be NULL. > >>> > >>> cheers, > >>> Gerd > >> > >> That isn't a problem. It will just say "null" when it is printed. > >> No crash or garbage text. > > > > Handling of '%s' with NULL is undefined by the standard. Linux > > glibc prints the string "null", but other implementations can > > crash. > > "null" is also printed on Mac OS X. > > Do you want dev->qdev.id replaced with this? (dev->qdev.id != NULL) ? dev->qdev.id : "null" Even better: Don't print "ID: ..." in the first place in case it is not present. cheers, Gerd
On Aug 28, 2015, at 9:50 AM, Gerd Hoffmann wrote: > On Fr, 2015-08-28 at 09:39 -0400, Programmingkid wrote: >> On Aug 28, 2015, at 9:35 AM, Daniel P. Berrange wrote: >> >>> On Fri, Aug 28, 2015 at 09:08:12AM -0400, Programmingkid wrote: >>>> >>>> On Aug 28, 2015, at 7:50 AM, Gerd Hoffmann wrote: >>>> >>>>> On Mo, 2015-08-24 at 15:13 -0400, Programmingkid wrote: >>>>>> + monitor_printf(mon, " Device %d.%d, Port %s, Speed %s >>>>>> Mb/s," >>>>>> + "Product %s, ID %s\n", >>>>>> + bus->busnr, dev->addr, port->path, >>>>>> + usb_speed(dev->speed), >>>>>> + dev->product_desc, dev->qdev.id); >>>>>> >>>>> >>>>> dev->qdev.id can be NULL. >>>>> >>>>> cheers, >>>>> Gerd >>>> >>>> That isn't a problem. It will just say "null" when it is printed. >>>> No crash or garbage text. >>> >>> Handling of '%s' with NULL is undefined by the standard. Linux >>> glibc prints the string "null", but other implementations can >>> crash. >> >> "null" is also printed on Mac OS X. >> >> Do you want dev->qdev.id replaced with this? (dev->qdev.id != NULL) ? dev->qdev.id : "null" > > Even better: Don't print "ID: ..." in the first place in case it is not > present. The ID is printed so if the user needs to remove the device, device_del <id> can be used. If the ID isn't printed here, where do you want it to be printed? "info device-id"?
On 28 August 2015 at 14:55, Programmingkid <programmingkidx@gmail.com> wrote: > On Aug 28, 2015, at 9:50 AM, Gerd Hoffmann wrote: >> Even better: Don't print "ID: ..." in the first place in case it is not >> present. > > The ID is printed so if the user needs to remove the device, device_del <id> can be used. > > If the ID isn't printed here, where do you want it to be printed? "info device-id"? Gerd means, "print nothing at all, rather than ', ID null'". There's no point in printing something which isn't actually an ID you can use to delete the device. thanks -- PMM
On Aug 28, 2015, at 12:07 PM, Peter Maydell wrote: > On 28 August 2015 at 14:55, Programmingkid <programmingkidx@gmail.com> wrote: >> On Aug 28, 2015, at 9:50 AM, Gerd Hoffmann wrote: >>> Even better: Don't print "ID: ..." in the first place in case it is not >>> present. >> >> The ID is printed so if the user needs to remove the device, device_del <id> can be used. >> >> If the ID isn't printed here, where do you want it to be printed? "info device-id"? > > Gerd means, "print nothing at all, rather than ', ID null'". > There's no point in printing something which isn't actually > an ID you can use to delete the device. Ok. I can make a patch that does that.
diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 5f39e1e..d2a726c 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -655,9 +655,11 @@ void hmp_info_usb(Monitor *mon, const QDict *qdict) dev = port->dev; if (!dev) continue; - monitor_printf(mon, " Device %d.%d, Port %s, Speed %s Mb/s, Product %s\n", - bus->busnr, dev->addr, port->path, usb_speed(dev->speed), - dev->product_desc); + monitor_printf(mon, " Device %d.%d, Port %s, Speed %s Mb/s," + "Product %s, ID %s\n", + bus->busnr, dev->addr, port->path, + usb_speed(dev->speed), + dev->product_desc, dev->qdev.id); } } }
Print the ID of each device when 'info usb' is used in the monitor. Signed-off-by: John Arbuckle <programmingkidx@gmail.com> --- This patch is easier to test when the qdev-monitor.c patch I also submitted is applied first. It can still be tested without it. Just run QEMU with the -usb option. Then go to the monitor and type "device_add usb-mouse,id=mouse-one". You will see the ID printed with the usb-mouse's info when you type 'info usb'. The ID is used by device_del to remove a device from QEMU. With this patch, the user can actually see the ID of the usb device making it possible to actually pick the right device to remove. hw/usb/bus.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)