On Sat, May 16, 2020 at 07:23:42AM +0800, Tang Bin wrote:
> Hi Alan & Colin：
>
> On 2020/5/16 1:21, Alan Stern wrote:
> > On Fri, May 15, 2020 at 05:54:53PM +0100, Colin King wrote:
> > > From: Colin Ian King <colin.king@xxxxxxxxxxxxx>
> > >
> > > The comparison of hcd->irq to less than zero for an error check will
> > > never be true because hcd->irq is an unsigned int. Fix this by
> > > assigning the int retval to the return of platform_get_irq and checking
> > > this for the -ve error condition and assigning hcd->irq to retval.
> > >
> > > Addresses-Coverity: ("Unsigned compared against 0")
> > > Fixes: c856b4b0fdb5 ("USB: EHCI: ehci-mv: fix error handling in mv_ehci_probe()")
> > > Signed-off-by: Colin Ian King <colin.king@xxxxxxxxxxxxx>
> > > ---
> > Thanks to Coverity for spotting this. Any reason why it didn't spot
> > exactly the same mistake in the ohci-da8xx.c driver?
>
> I just looked at the code and wondered whether it would be more appropriate
> to modify the header file "hcd.h". Since 'irq' might be an negative, why
> not just modify the variables in the 'struct usb_hcd', 'unsigned int
> irq'--> 'int irq'? After all, it's a public one.
I think the code in the drivers should be changed. The reason is if
platform_get_irq() returns a negative value, then that value should be
what the probe function returns.
Alan Stern
