Am I missing something, or does usb_control_msg do a completely
unnecessary kmalloc?

        The following patch changes usb_control_msg to allocate the
16-byte devrequest structure on the stack, saving CPU cycles, eliminating
a mode of failure and making the code more readable.  I suspect
that the additional 12 bytes of stack space that this change uses 
would also be used by gcc when deferring the stack unwinding from the
old kmalloc call until after usb_internal_control_msg, although I
have not bothered to compile the old code with "gcc -S" to verify
this.

        I am in the midst of fixing a bunch of other 2.5.x non-USB things
that will prevent me from testing this change for the next few days,
so I would appreciate it if others would test this change and apply it if
there are no problems.

        For readability, I am submitting this patch in the "non-unified"
context diff format.  Normally, unified diffs are much more readable,
but, in this case, the context diff format makes it much clearer.  I
can resubmit it as a unidiff if that is important.  The patch is against
linux-2.5.1-pre8, but should apply equally well to 2.4.x and the cvs tree.


-- 
Adam J. Richter     __     ______________   4880 Stevens Creek Blvd, Suite 104
[EMAIL PROTECTED]     \ /                  San Jose, California 95129-1034
+1 408 261-6630         | g g d r a s i l   United States of America
fax +1 408 261-6631      "Free Software For The Rest Of Us."
*** linux-2.5.1-pre8/drivers/usb/usb.c  Sat Dec  8 22:27:05 2001
--- linux/drivers/usb/usb.c     Mon Dec 10 22:59:44 2001
***************
*** 1319,1343 ****
  int usb_control_msg(struct usb_device *dev, unsigned int pipe, __u8 request, __u8 
requesttype,
                         __u16 value, __u16 index, void *data, __u16 size, int timeout)
  {
!       devrequest *dr = kmalloc(sizeof(devrequest), GFP_KERNEL);
!       int ret;
        
!       if (!dr)
!               return -ENOMEM;
! 
!       dr->requesttype = requesttype;
!       dr->request = request;
!       dr->value = cpu_to_le16p(&value);
!       dr->index = cpu_to_le16p(&index);
!       dr->length = cpu_to_le16p(&size);
  
        //dbg("usb_control_msg");       
  
!       ret = usb_internal_control_msg(dev, pipe, dr, data, size, timeout);
! 
!       kfree(dr);
! 
!       return ret;
  }
  
  
--- 1319,1335 ----
  int usb_control_msg(struct usb_device *dev, unsigned int pipe, __u8 request, __u8 
requesttype,
                         __u16 value, __u16 index, void *data, __u16 size, int timeout)
  {
!       devrequest dr;
        
!       dr.requesttype = requesttype;
!       dr.request = request;
!       dr.value = cpu_to_le16p(&value);
!       dr.index = cpu_to_le16p(&index);
!       dr.length = cpu_to_le16p(&size);
  
        //dbg("usb_control_msg");       
  
!       return usb_internal_control_msg(dev, pipe, &dr, data, size, timeout);
  }
  
  

Reply via email to