Subject: Re: [PATCH 1/2] ia64: topology: Fix an error handling path in cache_add_dev()

Hi Christophe!

On 3/28/22 21:07, Christophe JAILLET wrote:
> If kobject_init_and_add()fails, kobject_put() needs to be called.
> Add the missing call which is already there a few lines below in another
> error handling path.
>
> Fixes: f19180056ea0 ("[IA64] Export cpu cache info by sysfs")
> Signed-off-by: Christophe JAILLET <[email protected]>

Thanks for your patches. There is currently no maintainer for ia64, so the patches
would have to go through Andrew Morton's tree.

However, I can test the patches and verify they don't break anything.

Adrian

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913


2022-03-28 21:54:56

by Christophe JAILLET

[permalink] [raw]
Subject: Re: [PATCH 1/2] ia64: topology: Fix an error handling path in cache_add_dev()

Le 28/03/2022 à 21:16, John Paul Adrian Glaubitz a écrit :
> Hi Christophe!
>
> On 3/28/22 21:07, Christophe JAILLET wrote:
>> If kobject_init_and_add()fails, kobject_put() needs to be called.
>> Add the missing call which is already there a few lines below in another
>> error handling path.
>>
>> Fixes: f19180056ea0 ("[IA64] Export cpu cache info by sysfs")
>> Signed-off-by: Christophe JAILLET <[email protected]>
>
> Thanks for your patches. There is currently no maintainer for ia64, so the patches
> would have to go through Andrew Morton's tree.
>
> However, I can test the patches and verify they don't break anything.
>
> Adrian
>

Hi,

digging deeper for other potential same issues in other file, I don't
think that this patch is needed, and I don't think that it fixes anything.

The "name" of this kobject is "%s", "cache".
So nothing needs to be freed for that because kstrdup_const() will be used.

This kobject has no .release function.

If the add() part of kobject_init_and_add(), then 'state_in_sysfs' will
still be 0, so nothing needs to be released for that either.


So, adding a kobject_put() would just be a no-op here (if I understand
correctly).

I've been puzzled by the kobject_put() later, but in this case, _add()
has already succeeded and state_in_sysfs=1 and the call is needed.


For the other patch, it is just a clean-up. Based on Wikipedia, IA64 is
discontinued, so such clean-up does not make that much sense either.
(on the other hand, it should be eay to review and apply :) )


I don't think you need to spent time on it. Sorry for the noise.

CJ

Subject: Re: [PATCH 1/2] ia64: topology: Fix an error handling path in cache_add_dev()

Hi Christophe!

On 3/28/22 21:45, Christophe JAILLET wrote:
> For the other patch, it is just a clean-up. Based on Wikipedia, IA64 is
> discontinued, so such clean-up does not make that much sense either.
> (on the other hand, it should be eay to review and apply :) )

ia64 is still supported in Debian and Gentoo, so any kind of clean-up and
improvement of the kernel code is very welcome.

> I don't think you need to spent time on it. Sorry for the noise.

You didn't make any noise. If you have some improvements, I'm happy to test
them on my ia64 test system.

Adrian

--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913