Thank you for the review. And yes, I should have paid more attention to
the code, sorry. I have reflected the reviewed points in the V2 patch,
so please take a look at it.
On 12/5/24 22:20, Andy Shevchenko wrote:
> On Wed, Dec 04, 2024 at 09:21:52PM +0900, Joe Hattori wrote:
>> In gpiochip_setup_dev(), the refcount incremented in device_initialize()
>> is not decremented in the error path. Fix it by calling put_device().
>
> First of all, we have gpio_put_device().
Fixed in the V2 patch.
> Second, what the problem do you have in practice? Can you show any
backtrace?
> Third, how had this change been tested?
I am building a static analysis tool, and it reported this reference
leak. I overlooked that it still reported the same error after this
patch, and it is fixed in the V2 patch. If a backtrace is needed, I will
try.
>
> Looking at the current code I noticed the following:
> 1) gpiochip_add_data_with_key() has already that call;
Thank you for pointing out, the call has been removed in the V2 patch.
> 2) gpiochip_setup_devs() misses that call.
I was not sure if the gpiochip_setup_devs() should have an error path to
remove all the registered GPIO devices when a single registration fails,
thus it is not addressed in the V2 patch. If such a cleanup path is
needed, please let me know.
>
> This effectively means that you inroduce a regression while fixing a bug.
>
> The GPIO device initialisation is non-trivial, please pay more
attention to the
> code.
>
> Bart, can this be removed or reverted before it poisons stable?
>
Best,
Joe
