Re: [PATCH 01/26] usb: gadget: fsl: remove usage of list iterator past the loop body

From: Jakob Koschel
Date: Sun Mar 06 2022 - 14:19:54 EST




> On 6. Mar 2022, at 19:39, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Sun, Mar 6, 2022 at 9:51 AM Jakob Koschel <jakobkoschel@xxxxxxxxx> 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