On Wed, 4 Mar 2009, Greg KH wrote:
> Oh horrors.
>
> How about we just leave actual_length as int to allow this to work
> properly? Putting a negative value in an unsigned variable, while
> workable, is just a mess.
>
> How about the patch below instead for the whole thing?
I'm not so sure. It makes sense for actual_length to be unsigned, so
it ought to be changed.
The code in uhci-q.c doesn't have to use pseudo-negative values. I did
it that way because it was easy and because it would give the expected
output during debugging. But it doesn't have to work that way. Here's
another approach.
Alan Stern
Index: usb-2.6/drivers/usb/host/uhci-q.c
===================================================================
--- usb-2.6.orig/drivers/usb/host/uhci-q.c
+++ usb-2.6/drivers/usb/host/uhci-q.c
@@ -899,8 +899,6 @@ static int uhci_submit_control(struct uh
}
if (qh->state != QH_STATE_ACTIVE)
qh->skel = skel;
-
- urb->actual_length = -8; /* Account for the SETUP packet */
return 0;
nomem:
@@ -1494,11 +1492,10 @@ __acquires(uhci->lock)
if (qh->type == USB_ENDPOINT_XFER_CONTROL) {
- /* urb->actual_length < 0 means the setup transaction didn't
- * complete successfully. Either it failed or the URB was
- * unlinked first. Regardless, don't confuse people with a
- * negative length. */
- urb->actual_length = max(urb->actual_length, 0);
+ /* Subtract off the length of the SETUP packet from
+ * urb->actual_length.
+ */
+ urb->actual_length -= min_t(u32, 8, urb->actual_length);
}
/* When giving back the first URB in an Isochronous queue,
Index: usb-2.6/drivers/usb/host/uhci-debug.c
===================================================================
--- usb-2.6.orig/drivers/usb/host/uhci-debug.c
+++ usb-2.6/drivers/usb/host/uhci-debug.c
@@ -118,7 +118,9 @@ static int uhci_show_urbp(struct urb_pri
}
out += sprintf(out, "%s%s", ptype, (urbp->fsbr ? " FSBR" : ""));
- out += sprintf(out, " Actlen=%d", urbp->urb->actual_length);
+ out += sprintf(out, " Actlen=%d%s", urbp->urb->actual_length,
+ (urbp->qh->type == USB_ENDPOINT_XFER_CONTROL ?
+ "-8" : ""));
if (urbp->urb->unlinked)
out += sprintf(out, " Unlinked=%d", urbp->urb->unlinked);
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
