| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <1350704018.3764.240.camel@anish-Inspiron-N5050>
Date: Sat, 20 Oct 2012 12:33:38 +0900
From: anish kumar <anish198519851985@...il.com>
To: Chanwoo Choi <cw00.choi@...sung.com>
Cc: gregkh@...uxfoundation.org, myungjoo.ham@...sung.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [PATCH] extcon: driver model release call not needed
On Sat, 2012-10-20 at 11:37 +0900, Chanwoo Choi wrote:
> On 10/20/2012 11:30 AM, anish kumar wrote:
> > On Sat, 2012-10-20 at 10:57 +0900, Chanwoo Choi wrote:
> >> On 10/19/2012 02:12 AM, anish kumar wrote:
> >>> From: anish kumar <anish198519851985@...il.com>
> >>>
> >>> We don't need a release call in this file as we are doing
> >>> everything needed in unregister call and we don't have any
> >>> more pointer to free up.
> >>>
> >>> Signed-off-by: anish kumar <anish198519851985@...il.com>
> >>> ---
> >>> drivers/extcon/extcon-class.c | 4 +---
> >>> 1 files changed, 1 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> >>> index 946a318..cf30eb1 100644
> >>> --- a/drivers/extcon/extcon-class.c
> >>> +++ b/drivers/extcon/extcon-class.c
> >>> @@ -585,9 +585,7 @@ static void extcon_cleanup(struct extcon_dev *edev, bool skip)
> >>>
> >>> static void extcon_dev_release(struct device *dev)
> >>> {
> >>> - struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
> >>> -
> >>> - extcon_cleanup(edev, true);
> >>> + /* We don't have any thing to free here */
> >>> }
> >>>
> >>> static const char *muex_name = "mutually_exclusive";
> >>
> >> I can't agree this patch. The extcon_dev_release() function is used
> >> for dev->release. If some case without calling extcon_dev_unregister(),
> >> I think dev->release function is needed to free memory of edev->dev.
> > Is it not being released by extcon_dev_unregister?
> > I think it is released by that and we will do two times free and
> > list_del(&edev->entry) as it is called by extcon_dev_release also.
>
> I think that this patch should modify it as below patch to remove
> two call of kfree(). How about you?
>
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index e717bbc..efca0b4 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -597,9 +597,8 @@ static void extcon_cleanup(struct extcon_dev *edev, bool skip)
> #endif
> device_unregister(edev->dev);
> put_device(edev->dev);
> - }
> -
> - kfree(edev->dev);
> + } else {
> + kfree(edev->dev)
How about below?
diff --git a/drivers/extcon/extcon-class.c
b/drivers/extcon/extcon-class.c
index 946a318..62e4ecb 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -551,43 +551,9 @@ static int create_extcon_class(void)
return 0;
}
-static void extcon_cleanup(struct extcon_dev *edev, bool skip)
-{
- mutex_lock(&extcon_dev_list_lock);
- list_del(&edev->entry);
- mutex_unlock(&extcon_dev_list_lock);
-
- if (!skip && get_device(edev->dev)) {
- int index;
-
- if (edev->mutually_exclusive && edev->max_supported) {
- for (index = 0; edev->mutually_exclusive[index];
- index++)
-
kfree(edev->d_attrs_muex[index].attr.name);
- kfree(edev->d_attrs_muex);
- kfree(edev->attrs_muex);
- }
-
- for (index = 0; index < edev->max_supported; index++)
- kfree(edev->cables[index].attr_g.name);
-
- if (edev->max_supported) {
- kfree(edev->extcon_dev_type.groups);
- kfree(edev->cables);
- }
-
- device_unregister(edev->dev);
- put_device(edev->dev);
- }
-
- kfree(edev->dev);
-}
-
static void extcon_dev_release(struct device *dev)
{
- struct extcon_dev *edev = (struct extcon_dev *)
dev_get_drvdata(dev);
-
- extcon_cleanup(edev, true);
+ kfree(edev->dev);
}
static const char *muex_name = "mutually_exclusive";
@@ -813,7 +779,32 @@ EXPORT_SYMBOL_GPL(extcon_dev_register);
*/
void extcon_dev_unregister(struct extcon_dev *edev)
{
- extcon_cleanup(edev, false);
+ mutex_lock(&extcon_dev_list_lock);
+ list_del(&edev->entry);
+ mutex_unlock(&extcon_dev_list_lock);
+
+ if (!skip && get_device(edev->dev)) {
+ int index;
+
+ if (edev->mutually_exclusive && edev->max_supported) {
+ for (index = 0; edev->mutually_exclusive[index];
+ index++)
+
kfree(edev->d_attrs_muex[index].attr.name);
+ kfree(edev->d_attrs_muex);
+ kfree(edev->attrs_muex);
+ }
+
+ for (index = 0; index < edev->max_supported; index++)
+ kfree(edev->cables[index].attr_g.name);
+
+ if (edev->max_supported) {
+ kfree(edev->extcon_dev_type.groups);
+ kfree(edev->cables);
+ }
+
+ device_unregister(edev->dev);
+ put_device(edev->dev);
+ }
}
EXPORT_SYMBOL_GPL(extcon_dev_unregister);
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists