| Message ID | 20220306175034.3084609-1-jakobkoschel@gmail.com |
|---|---|
| Headers | show |
| Series | usb: gadget: remove usage of list iterator past the loop | expand |
> On 6. Mar 2022, at 19:39, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > On Sun, Mar 6, 2022 at 9:51 AM Jakob Koschel <jakobkoschel@gmail.com> wrote: >> >> /* make sure it's actually queued on this endpoint */ >> - list_for_each_entry(req, &ep->queue, queue) { >> - if (&req->req == _req) >> + list_for_each_entry(tmp, &ep->queue, queue) { >> + if (&tmp->req == _req) { >> + req = tmp; >> break; >> + } >> } > > Honestly, I think many (most?) of these would be a lot cleaner as > > list_for_each_entry(tmp, &ep->queue, queue) { > if (&tmp->req != _req) > continue; > req = tmp; > break; > } Alright, then I'll go ahead and adjust them. I tried keeping the code as similar as possible because in other cases it might be less cleaner inverting the condition. > > and in fact maybe that 'tmp' would be better named 'iter' or similar > (maybe 'pos', which is what the list.h macros themselves use for the > iterator naming), just from a naming standpoint. I agree, also here I simply kept it to what we concluded in the other thread. I also think using 'iter' would make more sense. > > Because it's not really some temporary variable, it has a real use. > > Linus Jakob
ok, let's start here. This patch set removes any use of the list iterator variable past the list body. This will allow defining the list iterator variable within the list_for_each_entry_*() macros to avoid any (invalid) use after the loop. If no break/goto was hit during list traversal the list iterator variable would otherwise be a bogus pointer since it is computed on something that is not actually an element of the list. I've basically followed what we discussed in: https://lore.kernel.org/all/YhdfEIwI4EdtHdym@kroah.com/ There are some cases where it might be possible to 'ditch' the tmp variable and refactor the code past the loop into the loop body. For the sake of keeping the *_dequeue() functions more similar, I've decided against doing it for some and leaving it in others. In general there are four use cases after the loop body here: 1) using &req->req in a comparision after the loop 2) using the iterator as a pointer in a comparision after the loop 3) use the &iterator->list to compare with the head to see if the loop exits early 4) using the iterator past the loop but using the rc variable to see if the loop exits early Jakob Koschel (26): usb: gadget: fsl: remove usage of list iterator past the loop body usb: gadget: bdc: remove usage of list iterator past the loop body usb: gadget: udc: atmel: remove usage of list iterator past the loop body usb: gadget: udc: pxa25x: remove usage of list iterator past the loop body usb: gadget: udc: at91: remove usage of list iterator past the loop body usb: gadget: goku_udc: remove usage of list iterator past the loop body usb: gadget: udc: gr_udc: remove usage of list iterator past the loop body usb: gadget: lpc32xx_udc: remove usage of list iterator past the loop body usb: gadget: mv_u3d: remove usage of list iterator past the loop body usb: gadget: udc: mv_udc_core: remove usage of list iterator past the loop body usb: gadget: net2272: remove usage of list iterator past the loop body usb: gadget: udc: net2280: remove usage of list iterator past the loop body usb: gadget: omap_udc: remove usage of list iterator past the loop body usb: gadget: s3c-hsudc: remove usage of list iterator past the loop body usb: gadget: udc-xilinx: remove usage of list iterator past the loop body usb: gadget: aspeed: remove usage of list iterator past the loop body usb: gadget: configfs: remove using list iterator after loop body as a ptr usb: gadget: legacy: remove using list iterator after loop body as a ptr usb: gadget: udc: max3420_udc: remove using list iterator after loop body as a ptr usb: gadget: tegra-xudc: remove using list iterator after loop body as a ptr usb: gadget: composite: remove check of list iterator against head past the loop body usb: gadget: pxa27x_udc: replace usage of rc to check if a list element was found usb: gadget: udc: s3c2410: replace usage of rc to check if a list element was found usb: gadget: udc: core: replace usage of rc to check if a list element was found usb: gadget: dummy_hcd: replace usage of rc to check if a list element was found usb: gadget: udc: s3c2410: replace usage of rc to check if a list element was found drivers/usb/gadget/composite.c | 18 ++++++++++-------- drivers/usb/gadget/configfs.c | 20 ++++++++++++-------- drivers/usb/gadget/legacy/hid.c | 23 ++++++++++++----------- drivers/usb/gadget/udc/aspeed-vhub/epn.c | 10 ++++++---- drivers/usb/gadget/udc/at91_udc.c | 10 ++++++---- drivers/usb/gadget/udc/atmel_usba_udc.c | 11 +++++++---- drivers/usb/gadget/udc/bdc/bdc_ep.c | 11 ++++++++--- drivers/usb/gadget/udc/core.c | 16 ++++++++++------ drivers/usb/gadget/udc/dummy_hcd.c | 11 ++++++----- drivers/usb/gadget/udc/fsl_qe_udc.c | 11 +++++++---- drivers/usb/gadget/udc/fsl_udc_core.c | 11 +++++++---- drivers/usb/gadget/udc/goku_udc.c | 10 ++++++---- drivers/usb/gadget/udc/gr_udc.c | 10 ++++++---- drivers/usb/gadget/udc/lpc32xx_udc.c | 10 ++++++---- drivers/usb/gadget/udc/max3420_udc.c | 11 +++++++---- drivers/usb/gadget/udc/mv_u3d_core.c | 10 ++++++---- drivers/usb/gadget/udc/mv_udc_core.c | 10 ++++++---- drivers/usb/gadget/udc/net2272.c | 11 ++++++----- drivers/usb/gadget/udc/net2280.c | 11 +++++++---- drivers/usb/gadget/udc/omap_udc.c | 10 ++++++---- drivers/usb/gadget/udc/pxa25x_udc.c | 11 +++++++---- drivers/usb/gadget/udc/pxa27x_udc.c | 9 +++++---- drivers/usb/gadget/udc/s3c-hsudc.c | 10 ++++++---- drivers/usb/gadget/udc/s3c2410_udc.c | 11 ++++++----- drivers/usb/gadget/udc/tegra-xudc.c | 10 ++++++---- drivers/usb/gadget/udc/udc-xilinx.c | 11 +++++++---- 26 files changed, 184 insertions(+), 123 deletions(-) base-commit: 719fce7539cd3e186598e2aed36325fe892150cf -- 2.25.1