Message ID | 1430212264-9672-3-git-send-email-arei.gonglei@huawei.com |
---|---|
State | New |
Headers | show |
28.04.2015 12:11, arei.gonglei@huawei.com wrote: > From: Gonglei <arei.gonglei@huawei.com> > > Value from xfer->packet.ep is assigned to ep here, but that > stored value is not used before it is overwritten. Remove it. > > Cc: Gerd Hoffmann <kraxel@redhat.com> > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > --- > hw/usb/hcd-xhci.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c > index ba15ae0..99f11fc 100644 > --- a/hw/usb/hcd-xhci.c > +++ b/hw/usb/hcd-xhci.c > @@ -2204,7 +2204,6 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, > if (epid == 1) { > if (xhci_fire_ctl_transfer(xhci, xfer) >= 0) { > epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE; > - ep = xfer->packet.ep; > } else { > DPRINTF("xhci: error firing CTL transfer\n"); > } This one is somewhat fun. ep variable is not used in whole this function until the very end, with the code: ep = xhci_epid_to_usbep(xhci, slotid, epid); if (ep) { usb_device_flush_ep_queue(ep->dev, ep); } There are only 2 assignments to it here, it is the NULL initializer and this place which is being removed by this patch (which is obviously unused). So, I think if we were to drop this assignment, we should remove the initializer too. But before doing this, I think we should try to remember _why_ this assignment is here in the first place. The code looks like after the loop, this ep variable was supposed to be used for something. Or is it just a leftover from 518ad5f2a075 (Cc'ing the author)? I haven't looked at all this in more details without good knowlege of the protocol and background. Thanks, /mjt
On 2015/4/29 14:48, Michael Tokarev wrote: > 28.04.2015 12:11, arei.gonglei@huawei.com wrote: >> From: Gonglei <arei.gonglei@huawei.com> >> >> Value from xfer->packet.ep is assigned to ep here, but that >> stored value is not used before it is overwritten. Remove it. >> >> Cc: Gerd Hoffmann <kraxel@redhat.com> >> Signed-off-by: Gonglei <arei.gonglei@huawei.com> >> --- >> hw/usb/hcd-xhci.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c >> index ba15ae0..99f11fc 100644 >> --- a/hw/usb/hcd-xhci.c >> +++ b/hw/usb/hcd-xhci.c >> @@ -2204,7 +2204,6 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, >> if (epid == 1) { >> if (xhci_fire_ctl_transfer(xhci, xfer) >= 0) { >> epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE; >> - ep = xfer->packet.ep; >> } else { >> DPRINTF("xhci: error firing CTL transfer\n"); >> } > > This one is somewhat fun. ep variable is not used in whole this > function until the very end, with the code: > > > ep = xhci_epid_to_usbep(xhci, slotid, epid); > if (ep) { > usb_device_flush_ep_queue(ep->dev, ep); > } > > There are only 2 assignments to it here, it is the NULL > initializer and this place which is being removed by this > patch (which is obviously unused). > > So, I think if we were to drop this assignment, we should > remove the initializer too. But before doing this, I think > we should try to remember _why_ this assignment is here in > the first place. The code looks like after the loop, this > ep variable was supposed to be used for something. Or is > it just a leftover from 518ad5f2a075 (Cc'ing the author)? > I can't agree with you more. :) Regards, -Gonglei > I haven't looked at all this in more details without good > knowlege of the protocol and background. > > Thanks, > > /mjt >
Hi, > > So, I think if we were to drop this assignment, we should > > remove the initializer too. But before doing this, I think > > we should try to remember _why_ this assignment is here in > > the first place. The code looks like after the loop, this > > ep variable was supposed to be used for something. Or is > > it just a leftover from 518ad5f2a075 (Cc'ing the author)? > > > I can't agree with you more. :) Yes, certainly looks like a 518ad5f2a075 leftover. Should be ok to cleanup. cheers, Gerd
Applied to -trivial, thanks! /mjt
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index ba15ae0..99f11fc 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2204,7 +2204,6 @@ static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid, if (epid == 1) { if (xhci_fire_ctl_transfer(xhci, xfer) >= 0) { epctx->next_xfer = (epctx->next_xfer + 1) % TD_QUEUE; - ep = xfer->packet.ep; } else { DPRINTF("xhci: error firing CTL transfer\n"); }