| Message ID | 20220306175034.3084609-26-jakobkoschel@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb: gadget: remove usage of list iterator past the loop | expand |
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@kroah.com/ > Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com> > --- > 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
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) { dev_dbg(udc_dev(dum), "dequeued req %p from %s, len %d buf %p\n", req, _ep->name, _req->length, _req->buf);
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@kroah.com/ Signed-off-by: Jakob Koschel <jakobkoschel@gmail.com> --- drivers/usb/gadget/udc/dummy_hcd.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)