Re: [PATCH 1/1] usb: xhci: fix build warning

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 15 Mar 2013, Peter Chen wrote:

> On Thu, Mar 14, 2013 at 10:59:45AM -0400, Alan Stern wrote:
> > On Thu, 14 Mar 2013, Peter Chen wrote:
> > 
> > > /home/b29397/work/code/git/linus/linux-2.6/drivers/usb/host/xhci-ring.c: In function ‘handle_port_status’:
> > > /home/b29397/work/code/git/linus/linux-2.6/drivers/usb/host/xhci-ring.c:1580: warning: ‘hcd’ may be used uninitialized in this function
> > > 
> > > Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx>
> > > ---
> > >  drivers/usb/host/xhci-ring.c |    4 ++--
> > >  1 files changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > > index 8828754..17dace0 100644
> > > --- a/drivers/usb/host/xhci-ring.c
> > > +++ b/drivers/usb/host/xhci-ring.c
> > > @@ -1588,6 +1588,8 @@ static void handle_port_status(struct xhci_hcd *xhci,
> > >  	__le32 __iomem **port_array;
> > >  	bool bogus_port_status = false;
> > >  
> > > +	/* Find the right roothub. */
> > > +	hcd = xhci_to_hcd(xhci);
> > >  	/* Port status change events always have a successful completion code */
> > >  	if (GET_COMP_CODE(le32_to_cpu(event->generic.field[2])) != COMP_SUCCESS) {
> > >  		xhci_warn(xhci, "WARN: xHC returned failed port status event\n");
> > > @@ -1629,8 +1631,6 @@ static void handle_port_status(struct xhci_hcd *xhci,
> > >  	 * into the index into the ports on the correct split roothub, and the
> > >  	 * correct bus_state structure.
> > >  	 */
> > > -	/* Find the right roothub. */
> > > -	hcd = xhci_to_hcd(xhci);
> > >  	if ((major_revision == 0x03) != (hcd->speed == HCD_USB3))
> > >  		hcd = xhci->shared_hcd;
> > 
> > You forgot to move these last two lines along with the first two.
> 
> No, major_revision is got above it.
> 
> In fact, the old logic is no problem due to flag bogus_port_status.

This doesn't matter.  Your change makes the code more fragile and
harder to understand, because in your version "hcd" is initialized in
two separate places.  The initialization should remain a single block
of code.

"Works okay" isn't good enough.  A lot of programmers seem to have
trouble absorbing this concept.  The code needs to be _clear_, because
it needs to continue to work correctly after other people make changes.  
If those other people can't understand it then they are likely to break
the code.

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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux