Simplify/refactor the way vendor/product/serial informations are

gathered from usb devices.

Cache them early in usbd_new_device() instead of querying the device
several times (the content wont change anyway) and uselessly generating
usb traffic (which could conflict with other access..)

The heuristic stays the same, first look in the device descriptor, then
try to match against usb_known_vendors/usb_known_products, then fallback
to use the idVendor/idProduct codes.

Remove the now useless 'usedev' parameter from usbd_fill_deviceinfo().

Tested from bsd & bsd.rd.

suggested by and ok mpi@

9 files changed, 78 insertions, 88 deletions

diff --git a/sys/dev/usb/ugen.c b/sys/dev/usb/ugen.c
index 673dc465807..4143a930e73 100644
--- a/sys/dev/usb/ugen.c
+++ b/sys/dev/usb/ugen.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ugen.c,v 1.97 2017/12/30 23:08:29 guenther Exp $ */
+/*	$OpenBSD: ugen.c,v 1.98 2018/05/01 18:14:46 landry Exp $ */
 /*	$NetBSD: ugen.c,v 1.63 2002/11/26 18:49:48 christos Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/ugen.c,v 1.26 1999/11/17 22:33:41 n_hibma Exp $	*/
 
@@ -1202,7 +1202,7 @@ ugen_do_ioctl(struct ugen_softc *sc, int endpt, u_long cmd, caddr_t addr,
 	}
 	case USB_GET_DEVICEINFO:
 		usbd_fill_deviceinfo(sc->sc_udev,
-				     (struct usb_device_info *)addr, 1);
+				     (struct usb_device_info *)addr);
 		break;
 	default:
 		return (EINVAL);
diff --git a/sys/dev/usb/uhid.c b/sys/dev/usb/uhid.c
index 547be4181d8..526f068b035 100644
--- a/sys/dev/usb/uhid.c
+++ b/sys/dev/usb/uhid.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uhid.c,v 1.70 2017/12/30 20:46:59 guenther Exp $ */
+/*	$OpenBSD: uhid.c,v 1.71 2018/05/01 18:14:46 landry Exp $ */
 /*	$NetBSD: uhid.c,v 1.57 2003/03/11 16:44:00 augustss Exp $	*/
 
 /*
@@ -364,7 +364,7 @@ uhid_do_ioctl(struct uhid_softc *sc, u_long cmd, caddr_t addr,
 
 	case USB_GET_DEVICEINFO:
 		usbd_fill_deviceinfo(sc->sc_hdev.sc_udev,
-				     (struct usb_device_info *)addr, 1);
+				     (struct usb_device_info *)addr);
 		break;
 
 	case USB_GET_REPORT_DESC:
diff --git a/sys/dev/usb/umass_scsi.c b/sys/dev/usb/umass_scsi.c
index b7757027136..d2f76f077d9 100644
--- a/sys/dev/usb/umass_scsi.c
+++ b/sys/dev/usb/umass_scsi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: umass_scsi.c,v 1.45 2016/08/03 13:44:49 krw Exp $ */
+/*	$OpenBSD: umass_scsi.c,v 1.46 2018/05/01 18:14:46 landry Exp $ */
 /*	$NetBSD: umass_scsipi.c,v 1.9 2003/02/16 23:14:08 augustss Exp $	*/
 /*
  * Copyright (c) 2001 The NetBSD Foundation, Inc.
@@ -160,7 +160,7 @@ umass_scsi_probe(struct scsi_link *link)
 	if (sc->maxlun > 0)
 		return (0);
 
-	usbd_fill_deviceinfo(sc->sc_udev, &udi, 1);
+	usbd_fill_deviceinfo(sc->sc_udev, &udi);
 
 	/*
 	 * Create a fake devid using the vendor and product ids and the last
diff --git a/sys/dev/usb/uoak_subr.c b/sys/dev/usb/uoak_subr.c
index 200fd384b11..6dfd5e8b292 100644
--- a/sys/dev/usb/uoak_subr.c
+++ b/sys/dev/usb/uoak_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uoak_subr.c,v 1.8 2017/01/09 14:44:28 mpi Exp $   */
+/*	$OpenBSD: uoak_subr.c,v 1.9 2018/05/01 18:14:46 landry Exp $   */
 
 /*
  * Copyright (c) 2012 Yojiro UO <yuo@nui.org>
@@ -231,7 +231,7 @@ void
 uoak_get_devinfo(struct uoak_softc *sc)
 {
 	/* get device serial# */
-	usbd_fill_deviceinfo(sc->sc_udev, &sc->sc_udi, 1);
+	usbd_fill_deviceinfo(sc->sc_udev, &sc->sc_udi);
 }
 
 void
diff --git a/sys/dev/usb/usb.c b/sys/dev/usb/usb.c
index 0c62cb2cb6a..d1c52bdd814 100644
--- a/sys/dev/usb/usb.c
+++ b/sys/dev/usb/usb.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: usb.c,v 1.118 2018/02/26 13:06:49 mpi Exp $	*/
+/*	$OpenBSD: usb.c,v 1.119 2018/05/01 18:14:46 landry Exp $	*/
 /*	$NetBSD: usb.c,v 1.77 2003/01/01 00:10:26 thorpej Exp $	*/
 
 /*
@@ -527,7 +527,7 @@ usb_fill_di_task(void *arg)
 	if (dev == NULL)
 		return;
 
-	usbd_fill_deviceinfo(dev, di, 0);
+	usbd_fill_deviceinfo(dev, di);
 }
 
 void
diff --git a/sys/dev/usb/usb_subr.c b/sys/dev/usb/usb_subr.c
index 558e393fa10..67ab10ff312 100644
--- a/sys/dev/usb/usb_subr.c
+++ b/sys/dev/usb/usb_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: usb_subr.c,v 1.135 2018/04/24 17:22:33 landry Exp $ */
+/*	$OpenBSD: usb_subr.c,v 1.136 2018/05/01 18:14:46 landry Exp $ */
 /*	$NetBSD: usb_subr.c,v 1.103 2003/01/10 11:19:13 augustss Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/usb_subr.c,v 1.18 1999/11/17 22:33:47 n_hibma Exp $	*/
 
@@ -66,6 +66,7 @@ char		*usbd_get_string(struct usbd_device *, int, char *, size_t);
 int		usbd_getnewaddr(struct usbd_bus *);
 int		usbd_print(void *, const char *);
 void		usbd_free_iface_data(struct usbd_device *, int);
+int		usbd_cache_devinfo(struct usbd_device *);
 usbd_status	usbd_probe_and_attach(struct device *,
 		    struct usbd_device *, int, int);
 
@@ -230,70 +231,70 @@ usbd_get_device_string(struct usbd_device *dev, uByte index)
 	return (buf);
 }
 
-void
-usbd_devinfo_vp(struct usbd_device *dev, char *v, size_t vl,
-    char *p, size_t pl, int usedev)
+int
+usbd_cache_devinfo(struct usbd_device *dev)
 {
 	usb_device_descriptor_t *udd = &dev->ddesc;
-	char *vendor = NULL, *product = NULL;
-#ifdef USBVERBOSE
-	const struct usb_known_vendor *ukv;
-	const struct usb_known_product *ukp;
-#endif
 
-	if (dev == NULL) {
-		v[0] = p[0] = '\0';
-		return;
-	}
+	dev->serial = malloc(USB_MAX_STRING_LEN, M_USB, M_NOWAIT);
+	if (dev->serial == NULL)
+		return (ENOMEM);
 
-	if (usedev) {
-		vendor = usbd_get_string(dev, udd->iManufacturer, v, vl);
-		usbd_trim_spaces(vendor);
-		product = usbd_get_string(dev, udd->iProduct, p, pl);
-		usbd_trim_spaces(product);
+	if (usbd_get_string(dev, udd->iSerialNumber, dev->serial, USB_MAX_STRING_LEN) != NULL) {
+		usbd_trim_spaces(dev->serial);
 	} else {
-		if (dev->vendor != NULL)
-			vendor = dev->vendor;
-		if (dev->product != NULL)
-			product = dev->product;
+		free(dev->serial, M_USB, USB_MAX_STRING_LEN);
+		dev->serial = NULL;
 	}
+
+	dev->vendor = malloc(USB_MAX_STRING_LEN, M_USB, M_NOWAIT);
+	if (dev->vendor == NULL)
+		return (ENOMEM);
+
+	if (usbd_get_string(dev, udd->iManufacturer, dev->vendor, USB_MAX_STRING_LEN) != NULL) {
+		usbd_trim_spaces(dev->vendor);
+	} else {
 #ifdef USBVERBOSE
-	if (vendor == NULL || product == NULL) {
-		for (ukv = usb_known_vendors;
-		    ukv->vendorname != NULL;
-		    ukv++) {
+		const struct usb_known_vendor *ukv;
+
+		for (ukv = usb_known_vendors; ukv->vendorname != NULL; ukv++) {
 			if (ukv->vendor == UGETW(udd->idVendor)) {
-				vendor = ukv->vendorname;
+				strlcpy(dev->vendor, ukv->vendorname,
+				    USB_MAX_STRING_LEN);
 				break;
 			}
 		}
-		if (vendor != NULL) {
-			for (ukp = usb_known_products;
-			    ukp->productname != NULL;
-			    ukp++) {
-				if (ukp->vendor == UGETW(udd->idVendor) &&
-				    (ukp->product == UGETW(udd->idProduct))) {
-					product = ukp->productname;
-					break;
-				}
+		if (ukv->vendorname == NULL)
+#endif
+			snprintf(dev->vendor, USB_MAX_STRING_LEN, "vendor 0x%04x",
+			    UGETW(udd->idVendor));
+	}
+
+	dev->product = malloc(USB_MAX_STRING_LEN, M_USB, M_NOWAIT);
+	if (dev->product == NULL)
+		return (ENOMEM);
+
+	if (usbd_get_string(dev, udd->iProduct, dev->product, USB_MAX_STRING_LEN) != NULL) {
+		usbd_trim_spaces(dev->product);
+	} else {
+#ifdef USBVERBOSE
+		const struct usb_known_product *ukp;
+
+		for (ukp = usb_known_products; ukp->productname != NULL; ukp++) {
+			if (ukp->vendor == UGETW(udd->idVendor) &&
+			    (ukp->product == UGETW(udd->idProduct))) {
+				strlcpy(dev->product, ukp->productname,
+				    USB_MAX_STRING_LEN);
+				break;
 			}
 		}
-	}
+		if (ukp->productname == NULL)
 #endif
+			snprintf(dev->product, USB_MAX_STRING_LEN, "product 0x%04x",
+			    UGETW(udd->idProduct));
+	}
 
-	if (v == vendor)
-		;
-	else if (vendor != NULL && *vendor)
-		strlcpy(v, vendor, vl);
-	else
-		snprintf(v, vl, "vendor 0x%04x", UGETW(udd->idVendor));
-
-	if (p == product)
-		;
-	else if (product != NULL && *product)
-		strlcpy(p, product, pl);
-	else
-		snprintf(p, pl, "product 0x%04x", UGETW(udd->idProduct));
+	return (0);
 }
 
 int
@@ -313,13 +314,10 @@ void
 usbd_devinfo(struct usbd_device *dev, int showclass, char *base, size_t len)
 {
 	usb_device_descriptor_t *udd = &dev->ddesc;
-	char vendor[USB_MAX_STRING_LEN];
-	char product[USB_MAX_STRING_LEN];
 	char *cp = base;
 	int bcdDevice, bcdUSB;
 
-	usbd_devinfo_vp(dev, vendor, sizeof vendor, product, sizeof product, 0);
-	snprintf(cp, len, "\"%s %s\"", vendor, product);
+	snprintf(cp, len, "\"%s %s\"", dev->vendor, dev->product);
 	cp += strlen(cp);
 	if (showclass) {
 		snprintf(cp, base + len - cp, ", class %d/%d",
@@ -1237,10 +1235,13 @@ usbd_new_device(struct device *parent, struct usbd_bus *bus, int depth,
 	DPRINTF(("usbd_new_device: new dev (addr %d), dev=%p, parent=%p\n",
 		 addr, dev, parent));
 
-	/* Cache some strings if possible. */
-	dev->vendor = usbd_get_device_string(dev, dev->ddesc.iManufacturer);
-	dev->product = usbd_get_device_string(dev, dev->ddesc.iProduct);
-	dev->serial = usbd_get_device_string(dev, dev->ddesc.iSerialNumber);
+	/* Get device info and cache it */
+	err = usbd_cache_devinfo(dev);
+	if (err) {
+		usb_free_device(dev);
+		up->device = NULL;
+		return (err);
+  	}
 
 	err = usbd_probe_and_attach(parent, dev, port, addr);
 	if (err) {
@@ -1300,16 +1301,15 @@ usbd_print(void *aux, const char *pnp)
 }
 
 void
-usbd_fill_deviceinfo(struct usbd_device *dev, struct usb_device_info *di,
-    int usedev)
+usbd_fill_deviceinfo(struct usbd_device *dev, struct usb_device_info *di)
 {
 	struct usbd_port *p;
 	int i, err, s;
 
 	di->udi_bus = dev->bus->usbctl->dv_unit;
 	di->udi_addr = dev->address;
-	usbd_devinfo_vp(dev, di->udi_vendor, sizeof(di->udi_vendor),
-	    di->udi_product, sizeof(di->udi_product), usedev);
+	strlcpy(di->udi_vendor, dev->vendor, sizeof(di->udi_vendor));
+	strlcpy(di->udi_product, dev->product, sizeof(di->udi_product));
 	usbd_printBCD(di->udi_release, sizeof di->udi_release,
 	    UGETW(dev->ddesc.bcdDevice));
 	di->udi_vendorNo = UGETW(dev->ddesc.idVendor);
@@ -1358,13 +1358,9 @@ usbd_fill_deviceinfo(struct usbd_device *dev, struct usb_device_info *di,
 		di->udi_nports = 0;
 
 	bzero(di->udi_serial, sizeof(di->udi_serial));
-	if (!usedev && dev->serial != NULL) {
+	if (dev->serial != NULL)
 		strlcpy(di->udi_serial, dev->serial,
 		    sizeof(di->udi_serial));
-	} else {
-		usbd_get_string(dev, dev->ddesc.iSerialNumber,
-		    di->udi_serial, sizeof(di->udi_serial));
-	}
 }
 
 /* Retrieve a complete descriptor for a certain device and index. */
diff --git a/sys/dev/usb/usbdi.h b/sys/dev/usb/usbdi.h
index 2f4848680a6..c16956592e4 100644
--- a/sys/dev/usb/usbdi.h
+++ b/sys/dev/usb/usbdi.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: usbdi.h,v 1.69 2017/05/15 11:05:51 mpi Exp $ */
+/*	$OpenBSD: usbdi.h,v 1.70 2018/05/01 18:14:46 landry Exp $ */
 /*	$NetBSD: usbdi.h,v 1.62 2002/07/11 21:14:35 augustss Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/usbdi.h,v 1.18 1999/11/17 22:33:49 n_hibma Exp $	*/
 
@@ -129,7 +129,7 @@ usb_config_descriptor_t *usbd_get_config_descriptor(struct usbd_device *dev);
 usb_device_descriptor_t *usbd_get_device_descriptor(struct usbd_device *dev);
 usbd_status usbd_set_interface(struct usbd_interface *, int);
 int usbd_get_no_alts(usb_config_descriptor_t *, int);
-void usbd_fill_deviceinfo(struct usbd_device *, struct usb_device_info *, int);
+void usbd_fill_deviceinfo(struct usbd_device *, struct usb_device_info *);
 usb_config_descriptor_t *usbd_get_cdesc(struct usbd_device *, int, u_int *);
 int usbd_get_interface_altindex(struct usbd_interface *iface);
 
diff --git a/sys/dev/usb/usbdivar.h b/sys/dev/usb/usbdivar.h
index 42f7974e90b..53c32f0153d 100644
--- a/sys/dev/usb/usbdivar.h
+++ b/sys/dev/usb/usbdivar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: usbdivar.h,v 1.74 2018/04/24 17:22:33 landry Exp $ */
+/*	$OpenBSD: usbdivar.h,v 1.75 2018/05/01 18:14:46 landry Exp $ */
 /*	$NetBSD: usbdivar.h,v 1.70 2002/07/11 21:14:36 augustss Exp $	*/
 /*	$FreeBSD: src/sys/dev/usb/usbdivar.h,v 1.11 1999/11/17 22:33:51 n_hibma Exp $	*/
 
@@ -257,8 +257,6 @@ usbd_status	usbd_fill_iface_data(struct usbd_device *, int, int);
 usbd_status	usb_insert_transfer(struct usbd_xfer *);
 void		usb_transfer_complete(struct usbd_xfer *);
 int		usbd_detach(struct usbd_device *, struct device *);
-void		usbd_devinfo_vp(struct usbd_device *, char *, size_t,
-		    char *, size_t, int);
 
 /* Routines from usb.c */
 void		usb_needs_explore(struct usbd_device *, int);
diff --git a/sys/dev/usb/uvideo.c b/sys/dev/usb/uvideo.c
index 56856a9d8d2..0c382b39071 100644
--- a/sys/dev/usb/uvideo.c
+++ b/sys/dev/usb/uvideo.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvideo.c,v 1.198 2018/04/28 03:13:04 visa Exp $ */
+/*	$OpenBSD: uvideo.c,v 1.199 2018/05/01 18:14:46 landry Exp $ */
 
 /*
  * Copyright (c) 2008 Robert Nagy <robert@openbsd.org>
@@ -2781,14 +2781,10 @@ int
 uvideo_querycap(void *v, struct v4l2_capability *caps)
 {
 	struct uvideo_softc *sc = v;
-	char vendor[USB_MAX_STRING_LEN];
-	char product[USB_MAX_STRING_LEN];
 
 	bzero(caps, sizeof(*caps));
 	strlcpy(caps->driver, DEVNAME(sc), sizeof(caps->driver));
-	usbd_devinfo_vp(sc->sc_udev, vendor, sizeof (vendor), product,
-	    sizeof (product), 0);
-	strlcpy(caps->card, product, sizeof(caps->card));
+	strlcpy(caps->card, sc->sc_udev->product, sizeof(caps->card));
 	strlcpy(caps->bus_info, "usb", sizeof(caps->bus_info));
 
 	caps->version = 1;