On Wed, 28 Aug 2013, Philippe De Swert wrote: > Since USB_SS_PORT_LS_U0 is 0x0000 the & operation with the port state would > always be 0. Thus the if would never be true. The code tries to check if the > port is enabled and link is active (U0 state). This means the port_status > should be 0x0001, so the right check is an | with USB_SS_PORT_LS_U0. > > Found with coverity: CID 744367, CID 145679 > > Signed-off-by: Philippe De Swert <philippe.deswert@xxxxxxxxxxxxxxx> > --- > drivers/usb/gadget/dummy_hcd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/dummy_hcd.c b/drivers/usb/gadget/dummy_hcd.c > index 5c506fb..22cd01c 100644 > --- a/drivers/usb/gadget/dummy_hcd.c > +++ b/drivers/usb/gadget/dummy_hcd.c > @@ -313,7 +313,7 @@ static void set_link_state_by_speed(struct dummy_hcd *dum_hcd) > (USB_PORT_STAT_C_CONNECTION << 16); > if ((dum_hcd->port_status & > USB_PORT_STAT_ENABLE) == 1 && > - (dum_hcd->port_status & > + (dum_hcd->port_status | > USB_SS_PORT_LS_U0) == 1 && > dum_hcd->rh_state != DUMMY_RH_SUSPENDED) > dum_hcd->active = 1; It's odd that Coverity found this bug but didn't find the bug immediately above: Since USB_PORT_STAT_ENABLE is 0x0002, the first comparison in this "if" statement can never succeed. The test against USB_PORT_STAT_ENABLE should be != 0, not == 1. The test against USB_SS_PORT_LS_U0 is dead code, because the driver currently does not support USB-3 suspend. Aside from that, the proposed fix is wrong. The correct test is (dum_hcd->port_status & USB_SS_PORT_LINK_STATE) == USB_SS_PORT_LS_U0 Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
