PERFORCE change 152749 for review

[Hans Petter Selasky]

http://perforce.freebsd.org/chv.cgi?CH=152749

Change 152749 by hselasky at hselasky_laptop001 on 2008/11/10 16:24:57

	
	Some fixes to USB string handling.
	
	1) Skip invalid characters instead of printing a dot.
	2) Make sure that the destination buffer is zero terminated in all error cases.
	3) Optimise usbconfig to use libusb20 when reading strings.

Affected files ...

.. //depot/projects/usb/src/lib/libusb20/libusb20.c#9 edit
.. //depot/projects/usb/src/sys/dev/usb2/core/usb2_request.c#20 edit
.. //depot/projects/usb/src/usr.sbin/usbconfig/dump.c#9 edit

Differences ...

==== //depot/projects/usb/src/lib/libusb20/libusb20.c#9 (text+ko) ====

@@ -712,26 +712,23 @@
 		/* too short buffer */
 		return (LIBUSB20_ERROR_INVALID_PARAM);
 	}
-	/*
-	 * Make sure that there is sensible contents in the buffer in case
-	 * of an error:
-	 */
-	*(uint8_t *)ptr = 0;
-
 	error = libusb20_dev_req_string_sync(pdev,
 	    0, 0, temp, sizeof(temp));
-	if (error < 0)
+	if (error < 0) {
+		*(uint8_t *)ptr = 0;	/* zero terminate */
 		return (error);
-
+	}
 	langid = temp[2] | (temp[3] << 8);
 
 	error = libusb20_dev_req_string_sync(pdev, strIndex,
 	    langid, temp, sizeof(temp));
-	if (error < 0)
+	if (error < 0) {
+		*(uint8_t *)ptr = 0;	/* zero terminate */
 		return (error);
-
+	}
 	if (temp[0] < 2) {
 		/* string length is too short */
+		*(uint8_t *)ptr = 0;	/* zero terminate */
 		return (LIBUSB20_ERROR_OTHER);
 	}
 	/* reserve one byte for terminating zero */
@@ -762,14 +759,16 @@
 			*buf = c >> 8;
 			swap = 2;
 		} else {
-			*buf = '.';
+			/* skip invalid character */
+			continue;
 		}
 		/*
 		 * Filter by default - we don't allow greater and less than
 		 * signs because they might confuse the dmesg printouts!
 		 */
 		if ((*buf == '<') || (*buf == '>') || (!isprint(*buf))) {
-			*buf = '.';
+			/* skip invalid character */
+			continue;
 		}
 		buf++;
 	}

==== //depot/projects/usb/src/sys/dev/usb2/core/usb2_request.c#20 (text+ko) ====

@@ -683,24 +683,26 @@
 		/* should not happen */
 		return (USB_ERR_NORMAL_COMPLETION);
 	}
-	buf[0] = 0;
-
 	if (string_index == 0) {
 		/* this is the language table */
+		buf[0] = 0;
 		return (USB_ERR_INVAL);
 	}
 	if (udev->flags.no_strings) {
+		buf[0] = 0;
 		return (USB_ERR_STALLED);
 	}
 	err = usb2_req_get_string_desc
 	    (udev, mtx, buf, len, udev->langid, string_index);
 	if (err) {
+		buf[0] = 0;
 		return (err);
 	}
 	temp = (uint8_t *)buf;
 
 	if (temp[0] < 2) {
 		/* string length is too short */
+		buf[0] = 0;
 		return (USB_ERR_INVAL);
 	}
 	/* reserve one byte for terminating zero */
@@ -732,7 +734,8 @@
 			*s = c >> 8;
 			swap = 2;
 		} else {
-			*s = '.';
+			/* silently skip bad character */
+			continue;
 		}
 
 		/*
@@ -740,11 +743,12 @@
 		 * signs because they might confuse the dmesg printouts!
 		 */
 		if ((*s == '<') || (*s == '>') || (!isprint(*s))) {
-			*s = '.';
+			/* silently skip bad character */
+			continue;
 		}
 		s++;
 	}
-	*s = 0;
+	*s = 0;				/* zero terminate resulting string */
 	return (USB_ERR_NORMAL_COMPLETION);
 }
 

==== //depot/projects/usb/src/usr.sbin/usbconfig/dump.c#9 (text+ko) ====

@@ -96,9 +96,6 @@
 dump_field(struct libusb20_device *pdev, const char *plevel,
     const char *field, uint32_t value)
 {
-	struct LIBUSB20_CONTROL_SETUP_DECODED req;
-	uint16_t lang_id;
-	uint8_t idx;
 	uint8_t temp_string[256];
 
 	printf("%s%s = 0x%04x ", plevel, field, value);
@@ -108,64 +105,15 @@
 		return;
 	}
 	if (value == 0) {
-		printf(" <no string> \n");
+		printf(" <no string>\n");
 		return;
 	}
-	LIBUSB20_INIT(LIBUSB20_CONTROL_SETUP, &req);
-
-	lang_id = 0;
-	idx = 0;
-
-	req.bmRequestType =
-	    LIBUSB20_REQUEST_TYPE_STANDARD |
-	    LIBUSB20_RECIPIENT_DEVICE |
-	    LIBUSB20_ENDPOINT_IN;
-	req.bRequest = LIBUSB20_REQUEST_GET_DESCRIPTOR;
-	req.wValue = (256 * LIBUSB20_DT_STRING) | idx;
-	req.wIndex = lang_id;
-	req.wLength = 4;		/* bytes */
-
-	if (libusb20_dev_request_sync(pdev, &req,
-	    temp_string, NULL, 1000, 0)) {
-		goto done;
+	if (libusb20_dev_req_string_simple_sync(pdev, value,
+	    temp_string, sizeof(temp_string))) {
+		printf(" <retrieving string failed>\n");
+		return;
 	}
-	lang_id = temp_string[2] | (temp_string[3] << 8);
-
-	printf(" LangId:0x%04x <", lang_id);
-
-	idx = value;
-
-	req.wValue = (256 * LIBUSB20_DT_STRING) | idx;
-	req.wIndex = lang_id;
-	req.wLength = 4;		/* bytes */
-
-	if (libusb20_dev_request_sync(pdev, &req,
-	    temp_string, NULL, 1000, 0)) {
-		printf("ERROR>\n");
-		goto done;
-	}
-	req.wValue = (256 * LIBUSB20_DT_STRING) | idx;
-	req.wIndex = lang_id;
-	req.wLength = temp_string[0];	/* bytes */
-
-	if (libusb20_dev_request_sync(pdev, &req,
-	    temp_string, NULL, 1000, 0)) {
-		printf("ERROR>\n");
-		goto done;
-	}
-	req.wLength /= 2;
-
-	for (idx = 1; idx != req.wLength; idx++) {
-		if (isprint(temp_string[(2 * idx) + 0])) {
-			printf("%c", temp_string[(2 * idx) + 0]);
-		} else if (isprint(temp_string[(2 * idx) + 1])) {
-			printf("%c", temp_string[(2 * idx) + 1]);
-		} else {
-			printf("?");
-		}
-	}
-	printf(">\n");
-done:
+	printf(" <%s>\n", temp_string);
 	return;
 }