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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 6 Mar 2022 20:19:46 +0100
From:   Jakob Koschel <jakobkoschel@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Greg Kroah-Hartman <greg@...ah.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Felipe Balbi <balbi@...nel.org>, Joel Stanley <joel@....id.au>,
        Andrew Jeffery <andrew@...id.au>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        Cristian Birsan <cristian.birsan@...rochip.com>,
        Al Cooper <alcooperx@...il.com>, Li Yang <leoyang.li@....com>,
        Vladimir Zapolskiy <vz@...ia.com>,
        Daniel Mack <daniel@...que.org>,
        Haojian Zhuang <haojian.zhuang@...il.com>,
        Robert Jarzmik <robert.jarzmik@...e.fr>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Alim Akhtar <alim.akhtar@...sung.com>,
        Thierry Reding <thierry.reding@...il.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Michal Simek <michal.simek@...inx.com>,
        "open list:USB GADGET/PERIPHERAL SUBSYSTEM" 
        <linux-usb@...r.kernel.org>, Mike Rapoport <rppt@...nel.org>,
        Brian Johannesmeyer <bjohannesmeyer@...il.com>,
        Cristiano Giuffrida <c.giuffrida@...nl>,
        "Bos, H.J." <h.j.bos@...nl>
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 <torvalds@...ux-foundation.org> wrote:
> 
> On Sun, Mar 6, 2022 at 9:51 AM Jakob Koschel <jakobkoschel@...il.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