[U-Boot] [PATCH] fsl/usb: fix phy_type checking

[Albert ARIBAUD]

Hi nikhil.badola at freescale.com,

On Mon, 17 Feb 2014 10:06:29 +0000, "nikhil.badola at freescale.com"
<nikhil.badola at freescale.com> wrote:

> Hi Albert,
> 
> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.u.boot at aribaud.net] 
> Sent: Monday, February 17, 2014 3:05 PM
> To: Badola Nikhil-B46172
> Cc: u-boot at lists.denx.de; Xie Shaohui-B21989
> Subject: Re: [U-Boot] [PATCH] fsl/usb: fix phy_type checking
> 
> Hi Nikhil,
> 
> On Mon, 17 Feb 2014 14:39:47 +0530, Nikhil Badola <nikhil.badola at freescale.com> wrote:
> 
> > Strcmp should not be used to check the argument of phy_type which 
> > maybe parsed by hwconfig_subarg. Hwconfig_subarg returns part of 
> > hwconfig starting from the argument (if it has the argument) till the 
> > end of the string. Since phy_type could be either 'utmi' or 'ulpi', 
> > strncmp should be used along with length limited to 4
> 
> Not sure I understand what exact problem you are considering here. If you know that phy_type is either "utmi" or "ulpi", then there is no benefit in switching from strcmp() to strncmp() since there is no risk that strcmp() overruns any of its arguments.

(seems like your mailer does not quote properly. Can you fix this?)

> [Nikhil Badola] :
> Strcmp() won't overrun any of its arguments but it does check NULL character at the end of both strings.
> Let me explain this with an example:
> Consider the hwconfig string to be defined as 
> hwconfig=usb1:dr_mode=host,phy_type=utmi;fsl_ddr:bank_intlv=auto
> 
> When hwconfig string is parsed for "phy_type" sub-argument, it returns string "utmi;fsl_ddr:bank_intlv=auto" which gets stored in phy_type pointer. When strcmp() is used, then '\0' character of "utmi" string is compared with ';' of phy_type hence returning non-zero value.

That is much clearer. Can you post a V2 with an amended commit message,
something like "limit phy_type comparison to the 4 first characters, so
that a comparison of "utmi;fsl_ddr:bank_intlv=auto" with "utmi" will
succeed".

> Amicalement,
> --
> Albert.

Amicalement,
-- 
Albert.