Skip to contentSkip to conversation

drivers/usb/misc/usbtest.c weirdness

PATCH Applied Marcin Slusarz 3 participants 4 messages May 11, 2008 – May 13, 2008 USB

lore
Share & tools
lore
#0 lore raw

test_ctrl_queue expects (?) positive and negative errnos. what is going on here?

drivers/usb/misc/usbtest.c +10 -10
index 742be3c..5a65991 100644
@@ -880,7 +880,7 @@ test_ctrl_queue (struct usbtest_dev *dev, struct usbtest_param *param)
 			req.bRequestType = USB_DIR_IN|USB_RECIP_INTERFACE;
 			// index = 0 means first interface
 			len = 1;
-			expected = EPIPE;
+			expected = EPIPE; //??
 			break;
 		case 3:		// get interface status
 			req.bRequest = USB_REQ_GET_STATUS;
@@ -897,7 +897,7 @@ test_ctrl_queue (struct usbtest_dev *dev, struct usbtest_param *param)
 			req.wValue = cpu_to_le16 (USB_DT_DEVICE_QUALIFIER << 8);
 			len = sizeof (struct usb_qualifier_descriptor);
 			if (udev->speed != USB_SPEED_HIGH)
-				expected = EPIPE;
+				expected = EPIPE; // ??
 			break;
 		case 6:		// get first config descriptor, plus interface
 			req.wValue = cpu_to_le16 ((USB_DT_CONFIG << 8) | 0);
@@ -908,7 +908,7 @@ test_ctrl_queue (struct usbtest_dev *dev, struct usbtest_param *param)
 			req.wValue = cpu_to_le16 (USB_DT_INTERFACE << 8);
 			// interface == 0
 			len = sizeof (struct usb_interface_descriptor);
-			expected = -EPIPE;
+			expected = -EPIPE; // ok
 			break;
 		// NOTE: two consecutive stalls in the queue here.
 		// that tests fault recovery a bit more aggressively.
@@ -919,7 +919,7 @@ test_ctrl_queue (struct usbtest_dev *dev, struct usbtest_param *param)
 			// wIndex 0 == ep0 (shouldn't halt!)
 			len = 0;
 			pipe = usb_sndctrlpipe (udev, 0);
-			expected = EPIPE;
+			expected = EPIPE; // ??
 			break;
 		case 9:		// get endpoint status
 			req.bRequest = USB_REQ_GET_STATUS;
@@ -930,14 +930,14 @@ test_ctrl_queue (struct usbtest_dev *dev, struct usbtest_param *param)
 		case 10:	// trigger short read (EREMOTEIO)
 			req.wValue = cpu_to_le16 ((USB_DT_CONFIG << 8) | 0);
 			len = 1024;
-			expected = -EREMOTEIO;
+			expected = -EREMOTEIO; // ok
 			break;
 		// NOTE: two consecutive _different_ faults in the queue.
 		case 11:	// get endpoint descriptor (ALWAYS STALLS)
 			req.wValue = cpu_to_le16 (USB_DT_ENDPOINT << 8);
 			// endpoint == 0
 			len = sizeof (struct usb_interface_descriptor);
-			expected = EPIPE;
+			expected = EPIPE; // ??
 			break;
 		// NOTE: sometimes even a third fault in the queue!
 		case 12:	// get string 0 descriptor (MAY STALL)
@@ -945,13 +945,13 @@ test_ctrl_queue (struct usbtest_dev *dev, struct usbtest_param *param)
 			// string == 0, for language IDs
 			len = sizeof (struct usb_interface_descriptor);
 			// may succeed when > 4 languages
-			expected = EREMOTEIO;	// or EPIPE, if no strings
+			expected = EREMOTEIO;	// or EPIPE, if no strings // ??
 			break;
 		case 13:	// short read, resembling case 10
 			req.wValue = cpu_to_le16 ((USB_DT_CONFIG << 8) | 0);
 			// last data packet "should" be DATA1, not DATA0
 			len = 1024 - udev->descriptor.bMaxPacketSize0;
-			expected = -EREMOTEIO;
+			expected = -EREMOTEIO; // ok
 			break;
 		case 14:	// short read; try to fill the last packet
 			req.wValue = cpu_to_le16 ((USB_DT_DEVICE << 8) | 0);
@@ -961,11 +961,11 @@ test_ctrl_queue (struct usbtest_dev *dev, struct usbtest_param *param)
 			case 8:		len = 24; break;
 			case 16:	len = 32; break;
 			}
-			expected = -EREMOTEIO;
+			expected = -EREMOTEIO; // ok
 			break;
 		default:
 			ERROR(dev, "bogus number of ctrl queue testcases!\n");
-			context.status = -EINVAL;
+			context.status = -EINVAL; // ok
 			goto cleanup;
 		}
 		req.wLength = cpu_to_le16 (len);
#1 lore raw
Quoted 3 lines · Marcin Slusarz

test_ctrl_queue expects (?) positive and negative errnos. what is going on here?

The sign is just a way to flag something:

                /* some faults are allowed, not required */

The negative ones are required. Positive codes are optional, in the sense that, depending on how the peripheral happens to be implemented, they won't necessarily be triggered.

For example, the test to fetch a device qualifier desriptor must succeed if the device is running at high speed. So that test is marked as negative. But when it's full speed, it could legitimately fail; marked as positive. And so on for other tests.

Look at how the codes are *interpreted* to see it work.

#2 lore raw

PATCH 1/1 usb/usbtest: comment on why this code "expects" negative and positive errnos

Quoted 13 lines · David Brownell
Quoted 3 lines · Marcin Slusarz

test_ctrl_queue expects (?) positive and negative errnos. what is going on here?

The sign is just a way to flag something:

                /* some faults are allowed, not required */

The negative ones are required. Positive codes are optional, in the sense that, depending on how the peripheral happens to be implemented, they won't necessarily be triggered.

Quoted 7 lines

For example, the test to fetch a device qualifier desriptor must succeed if the device is running at high speed. So that test is marked as negative. But when it's full speed, it could legitimately fail; marked as positive. And so on for other tests.

Look at how the codes are *interpreted* to see it work.

Thanks for a response!
Lets document it.

---
Based on comment from David Brownell <[email protected]>.

---
 drivers/usb/misc/usbtest.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
drivers/usb/misc/usbtest.c +5 -0
index 742be3c..054dedd 100644
@@ -856,6 +856,11 @@ test_ctrl_queue (struct usbtest_dev *dev, struct usbtest_param *param)
 		struct urb		*u;
 		struct usb_ctrlrequest	req;
 		struct subcase		*reqp;
+
+		/* sign of this variable means:
+		 *  -: tested code must return this (negative) error code
+		 *  +: tested code may return this (negative too) error code
+		 */
 		int			expected = 0;
 
 		/* requests here are mostly expected to succeed on any
Signature · 1 line
1.5.4.5
#3 lore raw

patch usbtest-comment-on-why-this-code-expects-negative-and-positive-errnos.patch added to gregkh-2.6 tree

This is a note to let you know that I've just added the patch titled

Subject: usbtest: comment on why this code "expects" negative and positive errnos

to my gregkh-2.6 tree. Its filename is

usbtest-comment-on-why-this-code-expects-negative-and-positive-errnos.patch

This tree can be found at
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/

Quoted 1 line

From [email protected] Tue May 13 14:58:09 2008

From: Marcin Slusarz <[email protected]>
Date: Mon, 12 May 2008 20:17:25 +0200
Subject: usbtest: comment on why this code "expects" negative and positive errnos
To: David Brownell <[email protected]>, LKML <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>, [email protected]
Message-ID: <20080512181721.GA6031@joi>
Content-Disposition: inline

Quoted 13 lines · David Brownell
Quoted 3 lines · Marcin Slusarz

test_ctrl_queue expects (?) positive and negative errnos. what is going on here?

The sign is just a way to flag something:

                /* some faults are allowed, not required */

The negative ones are required. Positive codes are optional, in the sense that, depending on how the peripheral happens to be implemented, they won't necessarily be triggered.

Quoted 7 lines

For example, the test to fetch a device qualifier desriptor must succeed if the device is running at high speed. So that test is marked as negative. But when it's full speed, it could legitimately fail; marked as positive. And so on for other tests.

Look at how the codes are *interpreted* to see it work.

Lets document it.

Based on comment from David Brownell <[email protected]>.

---
 drivers/usb/misc/usbtest.c |    5 +++++
 1 file changed, 5 insertions(+)
drivers/usb/misc/usbtest.c +5 -0
@@ -856,6 +856,11 @@ test_ctrl_queue (struct usbtest_dev *dev
 		struct urb		*u;
 		struct usb_ctrlrequest	req;
 		struct subcase		*reqp;
+
+		/* sign of this variable means:
+		 *  -: tested code must return this (negative) error code
+		 *  +: tested code may return this (negative too) error code
+		 */
 		int			expected = 0;
 
 		/* requests here are mostly expected to succeed on any

Patches currently in gregkh-2.6 which might be from [email protected] are

usb.current/usbtest-comment-on-why-this-code-expects-negative-and-positive-errnos.patch