On Sun, Mar 06, 2022 at 06:50:33PM +0100, Jakob Koschel wrote: > To move the list iterator variable into the list_for_each_entry_*() > macro in the future it should be avoided to use the list iterator > variable after the loop body. > > To *never* use the list iterator variable after the loop it was > concluded to use a separate iterator variable [1]. > > This removes the need to check the rc value to determine if the > break/goto was hit and can be made more obvious > by checking if the variable was set within the list traversal loop. > > Link: https://lore.kernel.org/all/YhdfEIwI4EdtHdym@xxxxxxxxx/ > Signed-off-by: Jakob Koschel <jakobkoschel@xxxxxxxxx> > --- > drivers/usb/gadget/udc/dummy_hcd.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/gadget/udc/dummy_hcd.c b/drivers/usb/gadget/udc/dummy_hcd.c > index a2d956af42a2..f21944707707 100644 > --- a/drivers/usb/gadget/udc/dummy_hcd.c > +++ b/drivers/usb/gadget/udc/dummy_hcd.c > @@ -751,7 +751,7 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req) > struct dummy *dum; > int retval = -EINVAL; > unsigned long flags; > - struct dummy_request *req = NULL; > + struct dummy_request *req = NULL, *tmp; > > if (!_ep || !_req) > return retval; > @@ -763,17 +763,18 @@ static int dummy_dequeue(struct usb_ep *_ep, struct usb_request *_req) > > local_irq_save(flags); > spin_lock(&dum->lock); > - list_for_each_entry(req, &ep->queue, queue) { > - if (&req->req == _req) { > - list_del_init(&req->queue); > + list_for_each_entry(tmp, &ep->queue, queue) { > + if (&tmp->req == _req) { > + list_del_init(&tmp->queue); > _req->status = -ECONNRESET; > + req = tmp; > retval = 0; > break; > } > } > spin_unlock(&dum->lock); > > - if (retval == 0) { > + if (req) { There's no need for this change as we are testing retval, not req here, unlike the other udc drivers. So this one I think is correct as-is, or am I mistaken somehow? thanks, greg k-h
