drivers/usb/misc/usbtest.c weirdness

drivers/usb/misc/usbtest.c weirdness

[Marcin Slusarz]

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

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 742be3c..5a65991 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -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);
-- 

Re: drivers/usb/misc/usbtest.c weirdness

[David Brownell]

On Sunday 11 May 2008, Marcin Slusarz wrote:
> 
> 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.

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

[Marcin Slusarz]

On Mon, May 12, 2008 at 01:02:22AM -0700, David Brownell wrote:
> On Sunday 11 May 2008, Marcin Slusarz wrote:
> > 
> > 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.

Thanks for a response!
Lets document it.

---
Based on comment from David Brownell <david-b@pacbell.net>.

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: David Brownell <david-b@pacbell.net>
---
 drivers/usb/misc/usbtest.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 742be3c..054dedd 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -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
-- 
1.5.4.5

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

[gregkh]

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/


>From marcin.slusarz@gmail.com  Tue May 13 14:58:09 2008
From: Marcin Slusarz <marcin.slusarz@gmail.com>
Date: Mon, 12 May 2008 20:17:25 +0200
Subject: usbtest: comment on why this code "expects" negative and positive errnos
To: David Brownell <david-b@pacbell.net>, LKML <linux-kernel@vger.kernel.org>
Cc: Greg Kroah-Hartman <gregkh@suse.de>, linux-usb@vger.kernel.org
Message-ID: <20080512181721.GA6031@joi>
Content-Disposition: inline


On Mon, May 12, 2008 at 01:02:22AM -0700, David Brownell wrote:
> On Sunday 11 May 2008, Marcin Slusarz wrote:
> > 
> > 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.

Lets document it.

Based on comment from David Brownell <david-b@pacbell.net>.

Signed-off-by: Marcin Slusarz <marcin.slusarz@gmail.com>
Cc: David Brownell <david-b@pacbell.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 drivers/usb/misc/usbtest.c |    5 +++++
 1 file changed, 5 insertions(+)

--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -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 marcin.slusarz@gmail.com are

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