Re: [PATCH v2 4/4] platform/x86: hp-bioscfg: Fix error handling in hp_add_other_attributes()

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1689 bytes --]

On Mon, 13 Nov 2023, Dan Carpenter wrote:

> On Mon, Nov 13, 2023 at 04:15:50PM +0200, Ilpo Järvinen wrote:
> > This relates to the 2nd problem (missing kobject_put()) and will be 
> > covered by the other patch. Don't try to solve this in the first patch
> > at all!
> > 
> > There are two indepedent problems:
> > - Before kobject_init_and_add(), kfree() is missing
> > - After kobject_init_and_add(), kobject_put() is missing
> 
> It's the same problem, though. The attr_name_kobj is leaked on all the
> error paths. 

I'll have politely disagree beyond that the symptoms are indeed about the 
same, the problem is clearly different like you immediately admit even 
yourself by stating this: ;-)

> It's just that it needs to be freed different ways depending on where 
> you are.

...And that's because "it" actually changed in between so the problem 
became a different one.

> To me splitting it up makes it harder to review

This has already been proven incorrect in the context of this patch so 
your argument is rather weak... While reviewing it I clearly noted that 
the different way of handling things was not properly covered, and that 
was because what needs to be "freed" was changed by 
kobject_init_and_add(). If one would have done them separately, each 
commit message would have been more to the point and it would have been 
simpler to review which is exactly the opposite to your claim. But I guess 
we'll end up disagreing on this too :-).

> and I would not allow it in Staging. You can't fix half the problem.

I don't have that strong opinion on this so Harshit please follow what 
Dan is suggesting, just fix the changelog to clearly cover both cases.


-- 
 i.