On Tue, 8 Mar 2005, David Brownell wrote:
> > Why not instead handle the control transfer case directly at
> > the spot where is_in is defined (there's a perfect opportunity a few lines
> > down) and then use it consistently?
>
> I'm not sure what you're saying there. "Where it's defined" is not
> the same as "a few lines down". And almost by definition, a single
> use is consistent (with itself) ...
>
>
> > Also you could make things slightly
> > simpler if is_in stored the direction mask rather than a boolean flag.
>
> If GCC is optimizing out its single use, it shouldn't matter ... ;)
I should have been more concrete. And I didn't notice that dr doesn't get
filled in until later.
How about something like the patch below?
Alan Stern
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
===== drivers/usb/core/devio.c 1.150 vs edited =====
--- 1.150/drivers/usb/core/devio.c 2005-03-02 11:39:56 -05:00
+++ edited/drivers/usb/core/devio.c 2005-03-08 13:30:39 -05:00
@@ -823,7 +823,7 @@
struct async *as;
struct usb_ctrlrequest *dr = NULL;
unsigned int u, totlen, isofrmlen;
- int ret, interval = 0, ifnum = -1;
+ int ret, interval = 0, ifnum = -1, is_in;
if (uurb->flags & ~(USBDEVFS_URB_ISO_ASAP|USBDEVFS_URB_SHORT_NOT_OK|
URB_NO_FSBR|URB_ZERO_PACKET))
@@ -832,13 +832,17 @@
return -EINVAL;
if (uurb->signr != 0 && (uurb->signr < SIGRTMIN || uurb->signr >
SIGRTMAX))
return -EINVAL;
- if (!(uurb->type == USBDEVFS_URB_TYPE_CONTROL && (uurb->endpoint &
~USB_ENDPOINT_DIR_MASK) == 0)) {
+ if (uurb->endpoint & ~(USB_ENDPOINT_DIR_MASK|USB_ENDPOINT_NUMBER_MASK))
+ return -EINVAL;
+ is_in = uurb->endpoint & USB_ENDPOINT_DIR_MASK;
+ if (uurb->type != USBDEVFS_URB_TYPE_CONTROL) {
if ((ifnum = findintfep(ps->dev, uurb->endpoint)) < 0)
return ifnum;
if ((ret = checkintf(ps, ifnum)))
return ret;
- }
- if ((uurb->endpoint & USB_ENDPOINT_DIR_MASK) != 0)
+ } else if ((uurb->endpoint & USB_ENDPOINT_NUMBER_MASK) != 0)
+ return -EINVAL;
+ if (is_in)
ep = ps->dev->ep_in [uurb->endpoint & USB_ENDPOINT_NUMBER_MASK];
else
ep = ps->dev->ep_out [uurb->endpoint &
USB_ENDPOINT_NUMBER_MASK];
@@ -866,11 +870,12 @@
kfree(dr);
return ret;
}
- uurb->endpoint = (uurb->endpoint & ~USB_ENDPOINT_DIR_MASK) |
(dr->bRequestType & USB_ENDPOINT_DIR_MASK);
+ is_in = uurb->endpoint = dr->bRequestType &
USB_ENDPOINT_DIR_MASK;
+ /* Warn if the direction changed? */
uurb->number_of_packets = 0;
uurb->buffer_length = le16_to_cpup(&dr->wLength);
uurb->buffer += 8;
- if (!access_ok((uurb->endpoint & USB_DIR_IN) ? VERIFY_WRITE :
VERIFY_READ, uurb->buffer, uurb->buffer_length)) {
+ if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
uurb->buffer, uurb->buffer_length)) {
kfree(dr);
return -EFAULT;
}
@@ -886,7 +891,7 @@
uurb->number_of_packets = 0;
if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
return -EINVAL;
- if (!access_ok((uurb->endpoint & USB_DIR_IN) ? VERIFY_WRITE :
VERIFY_READ, uurb->buffer, uurb->buffer_length))
+ if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
uurb->buffer, uurb->buffer_length))
return -EFAULT;
break;
@@ -930,7 +935,7 @@
interval = ep->desc.bInterval;
if (uurb->buffer_length > MAX_USBFS_BUFFER_SIZE)
return -EINVAL;
- if (!access_ok((uurb->endpoint & USB_DIR_IN) ? VERIFY_WRITE :
VERIFY_READ, uurb->buffer, uurb->buffer_length))
+ if (!access_ok(is_in ? VERIFY_WRITE : VERIFY_READ,
uurb->buffer, uurb->buffer_length))
return -EFAULT;
break;
@@ -971,14 +976,14 @@
kfree(isopkt);
as->ps = ps;
as->userurb = arg;
- if (uurb->endpoint & USB_DIR_IN)
+ if (is_in)
as->userbuffer = uurb->buffer;
else
as->userbuffer = NULL;
as->signr = uurb->signr;
as->ifnum = ifnum;
as->task = current;
- if (!(uurb->endpoint & USB_DIR_IN)) {
+ if (!is_in) {
if (copy_from_user(as->urb->transfer_buffer, uurb->buffer,
as->urb->transfer_buffer_length)) {
free_async(as);
return -EFAULT;
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
