Hi Doug,
On 2022-11-16, Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>> @@ -463,9 +464,14 @@ static void kgdboc_earlycon_pre_exp_handler(void)
>> * serial drivers might be OK with this, print a warning once per
>> * boot if we detect this case.
>> */
>> - for_each_console(con)
>> + cookie = console_srcu_read_lock();
>> + for_each_console_srcu(con) {
>> if (con == kgdboc_earlycon_io_ops.cons)
>> - return;
>> + break;
>> + }
>> + console_srcu_read_unlock(cookie);
>> + if (con)
>> + return;
>
> Is there truly any guarantee that "con" will be NULL if
> for_each_console_srcu() finishes naturally (AKA without a "break"
> being executed)?
Right now it is true because @con becoming NULL is the exit criteria for
the loop.
> It looks as if currently this will be true but nothing in the comments
> of for_each_console_srcu() nor hlist_for_each_entry_srcu() (which it
> calls) guarantees this, right? It would be nice if that was
> documented, but I guess it's not a huge deal.
Yes, if it is frowned upon that the iterator is used outside the loop,
it would be nice if the for_each macros explicitly provided some hints
in their documentation.
> Also: wasn't there just some big issue about people using loop
> iteration variables after the loop finished?
>
> https://lwn.net/Articles/885941/
Thanks for referencing that article! Indeed if the macros are changed so
that the iterator is defined in the loop, then code like this will
break. But I would expect that making such macro changes will also
require updating the call sites to avoid unused variables outside the
loops. And then this code could receive the appropriate fixup.
I feel like if I add extra code to guarantee a NULL without relying on
the macro implementation, I'll get more resistance due to unnecessarily
adding code and variables. But I may be wrong.
> Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
Thanks.
John
