From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vivek Gautam Date: Tue, 2 Apr 2013 12:28:12 +0530 Subject: [U-Boot] [PATCH 1/7] USB: Some cleanup prior to USB 3.0 interface addition In-Reply-To: <201304020846.36105.marex@denx.de> References: <1364376543-7526-1-git-send-email-gautam.vivek@samsung.com> <201304020826.50300.marex@denx.de> <201304020846.36105.marex@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On Tue, Apr 2, 2013 at 12:16 PM, Marek Vasut wrote: > Dear Vivek Gautam, > >> On Tue, Apr 2, 2013 at 11:56 AM, Marek Vasut wrote: >> > Dear Vivek Gautam, >> > >> >> Hi Marek, >> >> >> >> On Thu, Mar 28, 2013 at 7:56 PM, Marek Vasut wrote: >> >> > Dear Vivek Gautam, >> >> > >> >> >> Some cleanup in usb framework, nothing much on feature side. >> >> >> >> >> >> Signed-off-by: Vikas C Sajjan >> >> >> Signed-off-by: Vivek Gautam >> >> >> --- >> >> >> >> >> >> 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. -- Thanks & Regards Vivek