2013-03-19 17:08:56

by Alexandru Gheorghiu

[permalink] [raw]
Subject: [PATCH] drivers: video: omap2: dss: Use PTR_RET function

Use PTR_RET function instead of IS_ERR and PTR_ERR.
Patch found using coccinelle.

Signed-off-by: Alexandru Gheorghiu <[email protected]>
---
drivers/video/omap2/dss/core.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
index f8779d4..60cc6fe 100644
--- a/drivers/video/omap2/dss/core.c
+++ b/drivers/video/omap2/dss/core.c
@@ -181,10 +181,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
d = debugfs_create_file(name, S_IRUGO, dss_debugfs_dir,
write, &dss_debug_fops);

- if (IS_ERR(d))
- return PTR_ERR(d);
-
- return 0;
+ return PTR_RET(d);
}
#else /* CONFIG_OMAP2_DSS_DEBUGFS */
static inline int dss_initialize_debugfs(void)
--
1.7.9.5


2013-03-20 11:56:09

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH] drivers: video: omap2: dss: Use PTR_RET function

On 2013-03-19 10:03, Alexandru Gheorghiu wrote:
> Use PTR_RET function instead of IS_ERR and PTR_ERR.
> Patch found using coccinelle.
>
> Signed-off-by: Alexandru Gheorghiu <[email protected]>
> ---
> drivers/video/omap2/dss/core.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
> index f8779d4..60cc6fe 100644
> --- a/drivers/video/omap2/dss/core.c
> +++ b/drivers/video/omap2/dss/core.c
> @@ -181,10 +181,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
> d = debugfs_create_file(name, S_IRUGO, dss_debugfs_dir,
> write, &dss_debug_fops);
>
> - if (IS_ERR(d))
> - return PTR_ERR(d);
> -
> - return 0;
> + return PTR_RET(d);
> }
> #else /* CONFIG_OMAP2_DSS_DEBUGFS */
> static inline int dss_initialize_debugfs(void)
>

Thanks. I'll apply to omapdss tree.

Tomi



Attachments:
signature.asc (899.00 B)
OpenPGP digital signature

2013-03-20 15:44:35

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH] drivers: video: omap2: dss: Use PTR_RET function


On 03/20/2013 06:56 AM, Tomi Valkeinen wrote:
> On 2013-03-19 10:03, Alexandru Gheorghiu wrote:
>> Use PTR_RET function instead of IS_ERR and PTR_ERR.
>> Patch found using coccinelle.
>>
>> Signed-off-by: Alexandru Gheorghiu <[email protected]>
>> ---
>> drivers/video/omap2/dss/core.c | 5 +----
>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>
>> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
>> index f8779d4..60cc6fe 100644
>> --- a/drivers/video/omap2/dss/core.c
>> +++ b/drivers/video/omap2/dss/core.c
>> @@ -181,10 +181,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
>> d = debugfs_create_file(name, S_IRUGO, dss_debugfs_dir,
>> write, &dss_debug_fops);
>>
>> - if (IS_ERR(d))
>> - return PTR_ERR(d);
>> -
>> - return 0;
>> + return PTR_RET(d);
>> }
>> #else /* CONFIG_OMAP2_DSS_DEBUGFS */
>> static inline int dss_initialize_debugfs(void)
>>
>
> Thanks. I'll apply to omapdss tree.

Is this correct? If debugfs_create_file() returns a valid pointer, then
now dss_debugfs_create_file() will return a non-zero value on success. I
don't think this is what you want. A similar case came up recently here [1].

Jon

[1] http://comments.gmane.org/gmane.linux.kernel/1455141

2013-03-20 16:59:26

by Tomi Valkeinen

[permalink] [raw]
Subject: Re: [PATCH] drivers: video: omap2: dss: Use PTR_RET function

On 2013-03-20 17:44, Jon Hunter wrote:
>
> On 03/20/2013 06:56 AM, Tomi Valkeinen wrote:
>> On 2013-03-19 10:03, Alexandru Gheorghiu wrote:
>>> Use PTR_RET function instead of IS_ERR and PTR_ERR.
>>> Patch found using coccinelle.
>>>
>>> Signed-off-by: Alexandru Gheorghiu <[email protected]>
>>> ---
>>> drivers/video/omap2/dss/core.c | 5 +----
>>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
>>> index f8779d4..60cc6fe 100644
>>> --- a/drivers/video/omap2/dss/core.c
>>> +++ b/drivers/video/omap2/dss/core.c
>>> @@ -181,10 +181,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
>>> d = debugfs_create_file(name, S_IRUGO, dss_debugfs_dir,
>>> write, &dss_debug_fops);
>>>
>>> - if (IS_ERR(d))
>>> - return PTR_ERR(d);
>>> -
>>> - return 0;
>>> + return PTR_RET(d);
>>> }
>>> #else /* CONFIG_OMAP2_DSS_DEBUGFS */
>>> static inline int dss_initialize_debugfs(void)
>>>
>>
>> Thanks. I'll apply to omapdss tree.
>
> Is this correct? If debugfs_create_file() returns a valid pointer, then
> now dss_debugfs_create_file() will return a non-zero value on success. I
> don't think this is what you want. A similar case came up recently here [1].

Hmm. I don't follow. And I don't understand the post where you referred.

PTR_RET is defined as:

if (IS_ERR(ptr))
return PTR_ERR(ptr);
else
return 0;

How's that different from the original code?

Tomi



Attachments:
signature.asc (899.00 B)
OpenPGP digital signature

2013-03-20 18:02:33

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH] drivers: video: omap2: dss: Use PTR_RET function


On 03/20/2013 11:59 AM, Tomi Valkeinen wrote:
> On 2013-03-20 17:44, Jon Hunter wrote:
>>
>> On 03/20/2013 06:56 AM, Tomi Valkeinen wrote:
>>> On 2013-03-19 10:03, Alexandru Gheorghiu wrote:
>>>> Use PTR_RET function instead of IS_ERR and PTR_ERR.
>>>> Patch found using coccinelle.
>>>>
>>>> Signed-off-by: Alexandru Gheorghiu <[email protected]>
>>>> ---
>>>> drivers/video/omap2/dss/core.c | 5 +----
>>>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
>>>> index f8779d4..60cc6fe 100644
>>>> --- a/drivers/video/omap2/dss/core.c
>>>> +++ b/drivers/video/omap2/dss/core.c
>>>> @@ -181,10 +181,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
>>>> d = debugfs_create_file(name, S_IRUGO, dss_debugfs_dir,
>>>> write, &dss_debug_fops);
>>>>
>>>> - if (IS_ERR(d))
>>>> - return PTR_ERR(d);
>>>> -
>>>> - return 0;
>>>> + return PTR_RET(d);
>>>> }
>>>> #else /* CONFIG_OMAP2_DSS_DEBUGFS */
>>>> static inline int dss_initialize_debugfs(void)
>>>>
>>>
>>> Thanks. I'll apply to omapdss tree.
>>
>> Is this correct? If debugfs_create_file() returns a valid pointer, then
>> now dss_debugfs_create_file() will return a non-zero value on success. I
>> don't think this is what you want. A similar case came up recently here [1].
>
> Hmm. I don't follow. And I don't understand the post where you referred.
>
> PTR_RET is defined as:
>
> if (IS_ERR(ptr))
> return PTR_ERR(ptr);
> else
> return 0;
>
> How's that different from the original code?

Oops sorry, I missed that you were returning PTR_RET() and not
PTR_ERR(). Yes that looks fine. Sorry for the noise!

Jon

2013-03-20 18:08:32

by Jon Hunter

[permalink] [raw]
Subject: Re: [PATCH] drivers: video: omap2: dss: Use PTR_RET function


On 03/20/2013 01:02 PM, Jon Hunter wrote:
>
> On 03/20/2013 11:59 AM, Tomi Valkeinen wrote:
>> On 2013-03-20 17:44, Jon Hunter wrote:
>>>
>>> On 03/20/2013 06:56 AM, Tomi Valkeinen wrote:
>>>> On 2013-03-19 10:03, Alexandru Gheorghiu wrote:
>>>>> Use PTR_RET function instead of IS_ERR and PTR_ERR.
>>>>> Patch found using coccinelle.
>>>>>
>>>>> Signed-off-by: Alexandru Gheorghiu <[email protected]>
>>>>> ---
>>>>> drivers/video/omap2/dss/core.c | 5 +----
>>>>> 1 file changed, 1 insertion(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/video/omap2/dss/core.c b/drivers/video/omap2/dss/core.c
>>>>> index f8779d4..60cc6fe 100644
>>>>> --- a/drivers/video/omap2/dss/core.c
>>>>> +++ b/drivers/video/omap2/dss/core.c
>>>>> @@ -181,10 +181,7 @@ int dss_debugfs_create_file(const char *name, void (*write)(struct seq_file *))
>>>>> d = debugfs_create_file(name, S_IRUGO, dss_debugfs_dir,
>>>>> write, &dss_debug_fops);
>>>>>
>>>>> - if (IS_ERR(d))
>>>>> - return PTR_ERR(d);
>>>>> -
>>>>> - return 0;
>>>>> + return PTR_RET(d);
>>>>> }
>>>>> #else /* CONFIG_OMAP2_DSS_DEBUGFS */
>>>>> static inline int dss_initialize_debugfs(void)
>>>>>
>>>>
>>>> Thanks. I'll apply to omapdss tree.
>>>
>>> Is this correct? If debugfs_create_file() returns a valid pointer, then
>>> now dss_debugfs_create_file() will return a non-zero value on success. I
>>> don't think this is what you want. A similar case came up recently here [1].
>>
>> Hmm. I don't follow. And I don't understand the post where you referred.

Yes looking at Russell's response I am not sure I following now as it is
also using PTR_RET() and not PTR_ERR(). My eyes deceived me on this one.

Jon