* [PATCH] extcon: driver model release call not needed
@ 2012-10-21 4:46 anish kumar
2012-10-21 4:48 ` anish kumar
2012-10-22 0:55 ` Chanwoo Choi
0 siblings, 2 replies; 3+ messages in thread
From: anish kumar @ 2012-10-21 4:46 UTC (permalink / raw)
To: gregkh, myungjoo.ham, cw00.choi; +Cc: linux-kernel, anish kumar
From: anish kumar <anish198519851985@gmail.com>
There was a case where free and list_del can be called twice
on the same pointer.So fixed it by re-arranging the code and
removing a function which was not needed.
Signed-off-by: anish kumar <anish198519851985@gmail.com>
---
drivers/extcon/extcon-class.c | 71 ++++++++++++++++++-----------------------
1 files changed, 31 insertions(+), 40 deletions(-)
diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
index 754f133..347db10 100644
--- a/drivers/extcon/extcon-class.c
+++ b/drivers/extcon/extcon-class.c
@@ -544,47 +544,10 @@ 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);
- }
-
-#if defined(CONFIG_ANDROID)
- if (switch_class)
- class_compat_remove_link(switch_class, edev->dev, NULL);
-#endif
- 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);
+ struct extcon_dev *edev = dev_get_drvdata(dev);
+ kfree(edev->dev);
}
static const char *muex_name = "mutually_exclusive";
@@ -810,7 +773,35 @@ 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 (get_device(edev->dev) != NULL) {
+ 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);
+ }
+
+#if defined(CONFIG_ANDROID)
+ if (switch_class)
+ class_compat_remove_link(switch_class, edev->dev, NULL);
+#endif
+ device_unregister(edev->dev);
+ put_device(edev->dev);
+ }
}
EXPORT_SYMBOL_GPL(extcon_dev_unregister);
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] extcon: driver model release call not needed
2012-10-21 4:46 [PATCH] extcon: driver model release call not needed anish kumar
@ 2012-10-21 4:48 ` anish kumar
2012-10-22 0:55 ` Chanwoo Choi
1 sibling, 0 replies; 3+ messages in thread
From: anish kumar @ 2012-10-21 4:48 UTC (permalink / raw)
To: cw00.choi; +Cc: myungjoo.ham, linux-kernel, gregkh
Hi Choi,
This is based on http://git.infradead.org/users/kmpark/linux-samsung
this branch.
anish
On Sun, 2012-10-21 at 13:46 +0900, anish kumar wrote:
> From: anish kumar <anish198519851985@gmail.com>
>
> There was a case where free and list_del can be called twice
> on the same pointer.So fixed it by re-arranging the code and
> removing a function which was not needed.
>
> Signed-off-by: anish kumar <anish198519851985@gmail.com>
> ---
> drivers/extcon/extcon-class.c | 71 ++++++++++++++++++-----------------------
> 1 files changed, 31 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/extcon/extcon-class.c b/drivers/extcon/extcon-class.c
> index 754f133..347db10 100644
> --- a/drivers/extcon/extcon-class.c
> +++ b/drivers/extcon/extcon-class.c
> @@ -544,47 +544,10 @@ 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);
> - }
> -
> -#if defined(CONFIG_ANDROID)
> - if (switch_class)
> - class_compat_remove_link(switch_class, edev->dev, NULL);
> -#endif
> - 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);
> + struct extcon_dev *edev = dev_get_drvdata(dev);
> + kfree(edev->dev);
> }
>
> static const char *muex_name = "mutually_exclusive";
> @@ -810,7 +773,35 @@ 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 (get_device(edev->dev) != NULL) {
> + 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);
> + }
> +
> +#if defined(CONFIG_ANDROID)
> + if (switch_class)
> + class_compat_remove_link(switch_class, edev->dev, NULL);
> +#endif
> + device_unregister(edev->dev);
> + put_device(edev->dev);
> + }
> }
> EXPORT_SYMBOL_GPL(extcon_dev_unregister);
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] extcon: driver model release call not needed
2012-10-21 4:46 [PATCH] extcon: driver model release call not needed anish kumar
2012-10-21 4:48 ` anish kumar
@ 2012-10-22 0:55 ` Chanwoo Choi
1 sibling, 0 replies; 3+ messages in thread
From: Chanwoo Choi @ 2012-10-22 0:55 UTC (permalink / raw)
To: anish kumar; +Cc: gregkh, myungjoo.ham, linux-kernel
On 10/21/2012 01:46 PM, anish kumar wrote:
> From: anish kumar <anish198519851985@gmail.com>
>
> There was a case where free and list_del can be called twice
> on the same pointer.So fixed it by re-arranging the code and
> removing a function which was not needed.
>
> Signed-off-by: anish kumar <anish198519851985@gmail.com>
> ---
> drivers/extcon/extcon-class.c | 71 ++++++++++++++++++-----------------------
> 1 files changed, 31 insertions(+), 40 deletions(-)
>
Applied, thanks,
But, There were some minor issue, so I fix and applied it.
[...]
> static void extcon_dev_release(struct device *dev)
> {
> - struct extcon_dev *edev = (struct extcon_dev *) dev_get_drvdata(dev);
> -
> - extcon_cleanup(edev, true);
> + struct extcon_dev *edev = dev_get_drvdata(dev);
> + kfree(edev->dev);
It is not necessary to get extcon_dev instance through dev_get_drvdata()
and only call kfree(dev) by using extcon_dev_release()'s parameter.
kfree(dev);
> }
>
> static const char *muex_name = "mutually_exclusive";
> @@ -810,7 +773,35 @@ 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 (get_device(edev->dev) != NULL) {
I prefer minimal indentation, so I will modify as below code.
int index;
mutex_lock(&extcon_dev_list_lock);
list_del(&edev->entry);
mutex_unlock(&extcon_dev_list_lock);
if (IS_ERR_OR_NULL(get_device(edev->dev))) {
dev_err(edev->dev, "Failed to unregister extcon_dev (%s)\n",
dev_name(edev->dev));
return;
}
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);
}
[...]
Cheers,
Chanwoo Choi
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-10-22 0:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-21 4:46 [PATCH] extcon: driver model release call not needed anish kumar
2012-10-21 4:48 ` anish kumar
2012-10-22 0:55 ` Chanwoo Choi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.