Hi,
On Tue, Apr 2, 2013 at 12:16 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
On Tue, Apr 2, 2013 at 11:56 AM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Hi Marek,
On Thu, Mar 28, 2013 at 7:56 PM, Marek Vasut marex@denx.de wrote:
Dear Vivek Gautam,
Some cleanup in usb framework, nothing much on feature side.
Signed-off-by: Vikas C Sajjan vikas.sajjan@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
common/usb.c | 18 ++++++++++-------- common/usb_storage.c | 30 ++++++++++++++++-------------- include/usb_defs.h | 2 +- 3 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/common/usb.c b/common/usb.c index 6fc0fc1..40c1547 100644 --- a/common/usb.c +++ b/common/usb.c @@ -360,6 +360,7 @@ static int usb_parse_config(struct usb_device *dev,
int index, ifno, epno, curr_if_num; int i; u16 ep_wMaxPacketSize;
struct usb_interface *if_desc = NULL; ifno = -1; epno = -1;@@ -387,23 +388,24 @@ static int usb_parse_config(struct usb_device *dev,
&buffer[index])->bInterfaceNumber != curr_if_num) { /* this is a new interface, copy new desc */ ifno = dev->config.no_of_if;
if_desc = &dev->config.if_desc[ifno]; dev->config.no_of_if++;
memcpy(&dev->config.if_desc[ifno],&buffer[index], buffer[index]);dev->config.if_desc[ifno].no_of_ep = 0;dev->config.if_desc[ifno].num_altsetting = 1; + memcpy(if_desc, &buffer[index], buffer[index]); + if_desc->no_of_ep = 0; + if_desc->num_altsetting = 1;
curr_if_num =
dev-config.if_desc[ifno].desc.bInterfaceNumber;
if_desc->desc.bInterfaceNumber; } else { /* found alternate setting for the interface */
dev->config.if_desc[ifno].num_altsetting++; +
if_desc->num_altsetting++;
This will crash as if_desc is set in the if() branch above and it will be NULL in the else{} branch.
True, will add here if_desc = &dev->config.if_desc[ifno];
Yea ;-)
One more thing actually i could notice, "ifno = -1" in this else section. Is this what we want it here ?
Maybe I don't see it ?
ifno is initailized to -1. Now when descriptor is 'Interface type' (case USB_DT_INTERFACE) ifno is update in if part only as below: ifno = dev->config.no_of_if;
This would not be reaching else part, right ?
Ah I see, yep, indexing the array with -1 is no good. You can add a safety-check there , but the -1 should never reach that place indeed.
Ok, will amend this as required and update.