> On 28. Feb 2022, at 21:56, Christian König <christian.koenig@xxxxxxx> wrote:
>
>
>
> Am 28.02.22 um 21:42 schrieb James Bottomley:
>> On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote:
>>> Am 28.02.22 um 20:56 schrieb Linus Torvalds:
>>>> On Mon, Feb 28, 2022 at 4:19 AM Christian König
>>>> <christian.koenig@xxxxxxx> wrote:
>>>> [SNIP]
>>>> Anybody have any ideas?
>>> I think we should look at the use cases why code is touching (pos)
>>> after the loop.
>>>
>>> Just from skimming over the patches to change this and experience
>>> with the drivers/subsystems I help to maintain I think the primary
>>> pattern looks something like this:
>>>
>>> list_for_each_entry(entry, head, member) {
>>> if (some_condition_checking(entry))
>>> break;
>>> }
>>> do_something_with(entry);
There are other cases where the list iterator variable is used after the loop
Some examples:
- list_for_each_entry_continue() and list_for_each_entry_from().
- (although very rare) the head is actually of the correct struct type.
(ppc440spe_get_group_entry(): drivers/dma/ppc4xx/adma.c:1436)
- to use pos->list for example for list_add_tail():
(add_static_vm_early(): arch/arm/mm/ioremap.c:107)
If the scope of the list iterator is limited those still need fixing in a different way.
>>
>> Actually, we usually have a check to see if the loop found anything,
>> but in that case it should something like
>>
>> if (list_entry_is_head(entry, head, member)) {
>> return with error;
>> }
>> do_somethin_with(entry);
>>
>> Suffice? The list_entry_is_head() macro is designed to cope with the
>> bogus entry on head problem.
>
> That will work and is also what people already do.
>
> The key problem is that we let people do the same thing over and over again with slightly different implementations.
>
> Out in the wild I've seen at least using a separate variable, using a bool to indicate that something was found and just assuming that the list has an entry.
>
> The last case is bogus and basically what can break badly.
>
> If we would have an unified macro which search for an entry combined with automated reporting on patches to use that macro I think the potential to introduce such issues will already go down massively without auditing tons of existing code.
Having a unified way to do the same thing would indeed be great.
>
> Regards,
> Christian.
>
>>
>> James
>>
>>
>
- Jakob
