2022-03-06 22:50:54

by Jakob Koschel

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



> On 6. Mar 2022, at 19:39, Linus Torvalds <[email protected]> wrote:
>
> On Sun, Mar 6, 2022 at 9:51 AM Jakob Koschel <[email protected]> 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