2012-10-23 05:08:43

by Wei Yongjun

Subject: [PATCH] rtc: avoid calling platform_device_put() twice in test_init()

From: Wei Yongjun <[email protected]>

In case of error, the function test_init() need to call
platform_device_del() instead of platform_device_unregister().
Otherwise, we may call platform_device_put() twice.

dpatch engine is used to auto generate this patch.
(https://github.com/weiyj/dpatch)

Signed-off-by: Wei Yongjun <[email protected]>
---
drivers/rtc/rtc-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-test.c b/drivers/rtc/rtc-test.c
index 7e96254..209a127 100644
--- a/drivers/rtc/rtc-test.c
+++ b/drivers/rtc/rtc-test.c
@@ -164,7 +164,7 @@ static int __init test_init(void)
return 0;

exit_device_unregister:
- platform_device_unregister(test0);
+ platform_device_del(test0);

exit_free_test1:
platform_device_put(test1);

2012-11-07 00:51:46

by Andrew Morton

Subject: Re: [PATCH] rtc: avoid calling platform_device_put() twice in test_init()

On Tue, 23 Oct 2012 13:08:41 +0800
Wei Yongjun <[email protected]> wrote:

> From: Wei Yongjun <[email protected]>
>
> In case of error, the function test_init() need to call
> platform_device_del() instead of platform_device_unregister().
> Otherwise, we may call platform_device_put() twice.
>
> dpatch engine is used to auto generate this patch.
> (https://github.com/weiyj/dpatch)
>
> Signed-off-by: Wei Yongjun <[email protected]>
> ---
> drivers/rtc/rtc-test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-test.c b/drivers/rtc/rtc-test.c
> index 7e96254..209a127 100644
> --- a/drivers/rtc/rtc-test.c
> +++ b/drivers/rtc/rtc-test.c
> @@ -164,7 +164,7 @@ static int __init test_init(void)
> return 0;
>
> exit_device_unregister:
> - platform_device_unregister(test0);
> + platform_device_del(test0);
>
> exit_free_test1:
> platform_device_put(test1);

Is platform_device_del() the partner to platform_device_add()? If so
then yes, this looks right. But I think the labels can be improved:

--- a/drivers/rtc/rtc-test.c~rtc-avoid-calling-platform_device_put-twice-in-test_init-fix
+++ a/drivers/rtc/rtc-test.c
@@ -152,24 +152,24 @@ static int __init test_init(void)

if ((test1 = platform_device_alloc("rtc-test", 1)) == NULL) {
err = -ENOMEM;
- goto exit_free_test0;
+ goto exit_put_test0;
}

if ((err = platform_device_add(test0)))
- goto exit_free_test1;
+ goto exit_put_test1;

if ((err = platform_device_add(test1)))
- goto exit_device_unregister;
+ goto exit_del_test0;

return 0;

-exit_device_unregister:
+exit_del_test0:
platform_device_del(test0);

-exit_free_test1:
+exit_put_test1:
platform_device_put(test1);

-exit_free_test0:
+exit_put_test0:
platform_device_put(test0);

exit_driver_unregister:
_


However, take a look at test_exit(). it does
platform_device_unregister(test0) when test0 is in the same state. Is
that code wrong as well? Presumably it's working OK?

2012-11-08 06:13:48

by Wei Yongjun

Subject: Re: [PATCH] rtc: avoid calling platform_device_put() twice in test_init()

On 11/07/2012 08:51 AM, Andrew Morton wrote:
> On Tue, 23 Oct 2012 13:08:41 +0800
> Wei Yongjun <[email protected]> wrote:
>
>> From: Wei Yongjun <[email protected]>
>>
>> In case of error, the function test_init() need to call
>> platform_device_del() instead of platform_device_unregister().
>> Otherwise, we may call platform_device_put() twice.
>>
> Is platform_device_del() the partner to platform_device_add()? If so
> then yes, this looks right. But I think the labels can be improved:
>
> _
>
>
> However, take a look at test_exit(). it does
> platform_device_unregister(test0) when test0 is in the same state. Is
> that code wrong as well? Presumably it's working OK?

platform_device_unregister() is a warp of platform_device_del()
and platform_device_put(), if platform_device_add() success, we
can use platform_device_unregister() to do this, so that code
is OK.