> On 23. Feb 2022, at 21:22, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Feb 23, 2022 at 12:15 PM Jakob <jakobkoschel@xxxxxxxxx> wrote:
>>
>> in such a case you would still have to set the iterator value to
>> NULL when reaching the terminating condition or am I missing something?
>
> No.
>
> Make the rule be "you never use the iterator outside the loop".
>
> IOW, the code sequence is
>
> some_struct *ptr, *iter;
with C99 iter would be defined within the loop instead right?
>
> ptr = NULL;
> list_for_each_entry(iter, ...) {
> if (iter_matches_condition(iter)) {
> ptr = iter;
> break;
> }
> }
>
> .. never use 'iter' here - you use 'ptr' and check it for NULL ..
>
> See? Same number of variables as using a separate 'bool found' flag,
> but simpler code, and it matches the rule of 'don't use iter outside
> the loop'.
ah yes this does make sense. I missed the part of using a separate
'ptr' variable. Thanks for clarifying.
I think this is a great idea.
There are cases where pos->member is used (the only legitimate way to
use it right now). I suppose those turn into something like this
(this example is inspired by dev_add_offload() (net/core/gro.c:38)):
some_struct *ptr, *iter;
list_head *list_ptr;
ptr = NULL;
list_for_each_entry(iter, head, list) {
if (iter_matches_condition(iter)) {
ptr = iter;
break;
}
}
if (ptr)
list_ptr = head->prev;
else
list_ptr = iter->list.prev;
list_add(..., list_ptr);
before it was simply
list_add(..., iter->list.prev);
The other possibility I suppose would be:
if (!ptr)
ptr = container_of(head, typeof(*ptr), list)
list_add(..., ptr->list.prev);
which leaves you with the same type confusion as before, being far from
ideal.
> This is how you'd have to do it anyway if we start using a C99 style
> 'declare iter _in_ the loop' model.
>
> And as mentioned, it actually tends to lead to better code, since the
> code outside the loop only has one variable live, not two.
>
> Of course, compilers can do a lot of optimizations, so a 'found'
> variable can be made to generate good code too - if the compiler just
> tracks it and notices, and turns the 'break' into a 'goto found', and
> the fallthrough into the 'goto not_found'.
>
> So 'better code generation' is debatable, but even if the compiler can
> do as good a job with a separate 'bool' variable and some cleverness,
> I think we should strive for code where we make it easy for the
> compiler to DTRT - and where the generated code is easier to match up
> with what we wrote.
>
> Linus
If there is interest, I'm happy to send a new patch set once the fixes are clear.
Jakob
