Re: Re: [PATCH] gpio: rockchip: Fix missing of_node_put() in rockchip_gpio_probe() and rockchip_gpiolib_register()

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Wed, Jul 6, 2022 at 2:51 PM Liang He <windhl@xxxxxxx> wrote:
> At 2022-07-06 20:27:41, "Andy Shevchenko" <andy.shevchenko@xxxxxxxxx> wrote:
> >On Wed, Jul 6, 2022 at 8:29 AM Liang He <windhl@xxxxxxx> wrote:
> >>
> >> We should call of_node_put() for the reference returned by
> >> of_get_parent() which will increase the refcount.
> >
> >Is it suggested by the so-called Hulk Robot? If so, it's not the first
> >time I see that people don't think about, and just repeat as robots
> >do. Have you read the code? Have you tried to understand what may
> >happen when you put an OF node? What would be possible consequences to
> >the rest of the code?
> >
> >The above sentence is correct, implementation is not thought through.
> >It might be a correct fix, but the commit message doesn't show that
> >you really spent time on the change.
> >
> >P.S> I would personally put all those Hulk Robot bla-bla-bla with
> >lowest priority to consider, too many broken submissions...

> Thanks very much for you to review my another patch code.

You're welcome!

> First of all, all of my commits are not from Hulk Robot and I am sorry that you have spent lots of time
> for those broken submissions.
>
> Second, for this bug, while the of_get_parent() is only 3-4 lines above the adding of_node_put(),
> it is my fault to write so short commit log.

The problem is not in the length of the commit message, but in the
semantics of it.

The *put() methods that drop reference counting are part of the
garbage collector. If reference goes to 0 (or other defined threshold
for that purpose) the resource may be very well gone. Now, you should
check who and how is using that resource in the code. Depending on
that the patch may be good in its current form, or should be
completely rewritten.

-- 
With Best Regards,
Andy Shevchenko