On 6/3/2024 11:15 AM, Wolfram Sang wrote:
> There is a confusing pattern in the kernel to use a variable named 'timeout' to
> store the result of wait_event_timeout() causing patterns like:
>
> timeout = wait_event_timeout(...)
> if (!timeout) return -ETIMEDOUT;
>
> with all kinds of permutations. Use 'time_left' as a variable to make the code
> self explaining.
I feel this type of changes fall into the category of bike-shedding.
People should know how wait_event_timeout() works and then a variable
name does not really matter.
> Fix to the proper variable type 'long' while here.
But it may have useful side-effects to go over the code with fresh look.
Thanks.
Acked-by: Arend van Spriel <[email protected]>
> Signed-off-by: Wolfram Sang <[email protected]>
> ---
> .../wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
> I feel this type of changes fall into the category of bike-shedding. People
I have two motivations for this change: One may be bike-shedding, yet
"if (!timeout) return -ETIMEDOUT" looks stupid to me.
> should know how wait_event_timeout() works and then a variable name does not
> really matter.
And for a new developer, I am quite sure the change will help to
understand how wait_event-family() works. Especially given that
wait_event_interruptible() returns 0 if condition is true and
wait_event_interruptible_timeout() returns 0 if condition is false.
> > Fix to the proper variable type 'long' while here.
>
> But it may have useful side-effects to go over the code with fresh look.
:)
> Acked-by: Arend van Spriel <[email protected]>
Thank you!