Re: [PATCH v3 2/5] reset: Instantiate reset GPIO controller for shared reset-gpios

[Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>]

On 15/01/2024 18:32, Philipp Zabel wrote:
> On Mo, 2024-01-15 at 17:13 +0100, Krzysztof Kozlowski wrote:
>> On 15/01/2024 17:06, Bartosz Golaszewski wrote:
>>>> +
>>>> +static int __reset_add_reset_gpio_lookup(int id, struct device_node *np,
>>>> +                                        unsigned int gpio,
>>>> +                                        unsigned int of_flags)
>>>> +{
>>>> +       struct gpiod_lookup_table *lookup __free(kfree) = NULL;
>>>> +       struct gpio_device *gdev __free(gpio_device_put) = NULL;
>>>> +       char *label __free(kfree) = NULL;
>>>
>>> I got yelled at by Linus Torvalds personally for doing it like this. I
>>> know this is a common pattern in code using GLib but Linus wants auto
>>> variables to be initialized where they're declared...
>>
>> Declaration is here. Initialization is here. Therefore this is
>> initialized where it is declared. What's more it is initialized to a
>> valid value, because __free() accepts NULLs.
> [...]
>>> ... so this should become:
>>>
>>>   struct gpio_device *gdev __free(gpio_device_put) = gpio_device_find(...)
>>>
>>> and same for the rest.
>>>
>>> Don't get me wrong, I love cleanup.h but there's a (unofficial for
>>> now) coding style.
>>
>> So you just want to declare it not in top-part of the function but just
>> before first use?
> 
> IIUC, Linus wants exactly this:
> 
> https://lore.kernel.org/all/CAHk-=wgRHiV5VSxtfXA4S6aLUmcQYEuB67u3BJPJPtuESs1JyA@xxxxxxxxxxxxxx/

Sure.



Best regards,
Krzysztof