Messages in this thread | | | Date | Wed, 1 Mar 2023 22:44:06 -0500 | Subject | Re: [PATCH v2 2/4] extcon: Added extcon_alloc_cables to simplify extcon register function | From | Slade's Kernel Patch Bot <> |
| |
On 3/1/23 20:38, Bumwoo Lee wrote: > Hello. > > As you can see, edev->cables are freed if extcon_alloc_cables() function return error handling in extcon_dev_register() > Other added functions are also same. > > Because it's functionalized, apart from this, do you want to mention that it should be freed within the function? > Please let me know your opinion. > > extcon_dev_register(struct extcon_dev *edev){ > ... > > ret = extcon_alloc_cables(edev); > if (ret) > goto err_alloc_cables; > > ... > > err_alloc_cables: > if (edev->max_supported) > kfree(edev->cables); > > > Regards, > Bumwoo
This is Slade's kernel patch bot. When scanning his mailbox, I came across this message, which appears to be a top-post. Please do not top-post on Linux mailing lists.
A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail?
Please bottom-post to Linux mailing lists in the future. See also: https://daringfireball.net/2007/07/on_top
If you believe this is an error, please address a message to Slade Watkins <srw@sladewatkins.net>.
Thank you, -- Slade's kernel patch bot
> > -----Original Message----- > From: MyungJoo Ham <myungjoo.ham@samsung.com> > Sent: Friday, February 24, 2023 7:03 PM > To: Bumwoo Lee <bw365.lee@samsung.com>; Chanwoo Choi <cw00.choi@samsung.com>; linux-kernel@vger.kernel.org > Subject: RE: [PATCH v2 2/4] extcon: Added extcon_alloc_cables to simplify extcon register function > >> --------- Original Message --------- >> Sender : 이범우 <bw365.lee@samsung.com>Product S/W Lab(VD)/삼성전자 Date : >> 2023-02-20 14:45 (GMT+9) Title : [PATCH v2 2/4] extcon: Added >> extcon_alloc_cables to simplify extcon register function >> >> The cable allocation part is functionalized from extcon_dev_register. >> >> Signed-off-by: Bumwoo Lee <bw365.lee@samsung.com> >> --- >> drivers/extcon/extcon.c | 104 +++++++++++++++++++++++----------------- >> 1 file changed, 59 insertions(+), 45 deletions(-) >> >> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c index >> adcf01132f70..3c2f540785e8 100644 >> --- a/drivers/extcon/extcon.c >> +++ b/drivers/extcon/extcon.c >> @@ -1070,6 +1070,61 @@ void extcon_dev_free(struct extcon_dev *edev) } >> EXPORT_SYMBOL_GPL(extcon_dev_free); >> >> +/** >> + * extcon_alloc_cables() - alloc the cables for extcon device >> + * @edev: extcon device which has cables >> + * >> + * Returns 0 if success or error number if fail. >> + */ >> +static int extcon_alloc_cables(struct extcon_dev *edev) { >> + int index; >> + char *str; >> + struct extcon_cable *cable; >> + >> + if (!edev->max_supported) >> + return 0; >> + >> + edev->cables = kcalloc(edev->max_supported, >> + sizeof(struct extcon_cable), >> + GFP_KERNEL); >> + if (!edev->cables) >> + return -ENOMEM; >> + >> + for (index = 0; index < edev->max_supported; index++) { >> + cable = &edev->cables[index]; >> + >> + str = kasprintf(GFP_KERNEL, "cable.%d", index); >> + if (!str) { >> + for (index--; index >= 0; index--) { >> + cable = &edev->cables[index]; >> + kfree(cable->attr_g.name); >> + } >> + return -ENOMEM; > > You have a memory leak. > edev->cables is allocated and > you are not freeing it. > > In the previous code, it was freed by > having different err-goto labels. > > Please check if you have similar errors > in other patches of this series. > > ... > >> @@ -1282,7 +1296,7 @@ int extcon_dev_register(struct extcon_dev *edev) >> err_alloc_cables: >> if (edev->max_supported) >> kfree(edev->cables); >> -err_sysfs_alloc: >> + >> return ret; >> } >> EXPORT_SYMBOL_GPL(extcon_dev_register); >> -- >> 2.35.1 >> >> > > Cheers, > MyungJoo. > > >
| |