[PATCH] x86/intel/quark: fix ptr_ret.cocci warnings

[PATCH] x86/intel/quark: fix ptr_ret.cocci warnings

[kbuild test robot]

arch/x86/platform/intel-quark/imr.c:280:1-3: WARNING: PTR_ERR_OR_ZERO can be used


 Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Generated by: scripts/coccinelle/api/ptr_ret.cocci

CC: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 imr.c |    5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

--- a/arch/x86/platform/intel-quark/imr.c
+++ b/arch/x86/platform/intel-quark/imr.c
@@ -277,10 +277,7 @@ static int imr_debugfs_register(struct i
 {
 	idev->file = debugfs_create_file("imr_state", S_IFREG | S_IRUGO, NULL,
 					 idev, &imr_state_ops);
-	if (IS_ERR(idev->file))
-		return PTR_ERR(idev->file);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(idev->file);
 }
 
 /**

Re: [PATCH] x86/intel/quark: fix ptr_ret.cocci warnings

[Bryan O'Donoghue]

On 19/02/15 08:14, kbuild test robot wrote:
> arch/x86/platform/intel-quark/imr.c:280:1-3: WARNING: PTR_ERR_OR_ZERO can be used
>
>
>   Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR
>
> Generated by: scripts/coccinelle/api/ptr_ret.cocci
>
> CC: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
>
>   imr.c |    5 +----
>   1 file changed, 1 insertion(+), 4 deletions(-)
>
> --- a/arch/x86/platform/intel-quark/imr.c
> +++ b/arch/x86/platform/intel-quark/imr.c
> @@ -277,10 +277,7 @@ static int imr_debugfs_register(struct i
>   {
>   	idev->file = debugfs_create_file("imr_state", S_IFREG | S_IRUGO, NULL,
>   					 idev, &imr_state_ops);
> -	if (IS_ERR(idev->file))
> -		return PTR_ERR(idev->file);
> -
> -	return 0;
> +	return PTR_ERR_OR_ZERO(idev->file);
>   }
>
>   /**
>

This is a reasonable change.

ACK

[tip:x86/platform] x86/intel/quark: Fix ptr_ret.cocci warnings

[tip-bot for Fengguang Wu]

Commit-ID:  32d39169d7f56849b8c6c8c51aca7b73194d05f1
Gitweb:     http://git.kernel.org/tip/32d39169d7f56849b8c6c8c51aca7b73194d05f1
Author:     Fengguang Wu <fengguang.wu@intel.com>
AuthorDate: Thu, 19 Feb 2015 16:14:32 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 19 Feb 2015 10:00:54 +0100

x86/intel/quark: Fix ptr_ret.cocci warnings

arch/x86/platform/intel-quark/imr.c:280:1-3: WARNING: PTR_ERR_OR_ZERO can be used

 Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR

Generated by: scripts/coccinelle/api/ptr_ret.cocci

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Cc: Andy Shevchenko <andy.schevchenko@gmail.com>
Cc: Ong, Boon Leong <boon.leong.ong@intel.com>
Cc: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Darren Hart <dvhart@linux.intel.com>
Cc: kbuild-all@01.org
Link: http://lkml.kernel.org/r/20150219081432.GA21983@waimea
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/platform/intel-quark/imr.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/platform/intel-quark/imr.c b/arch/x86/platform/intel-quark/imr.c
index 16e4df1..60c01eb 100644
--- a/arch/x86/platform/intel-quark/imr.c
+++ b/arch/x86/platform/intel-quark/imr.c
@@ -277,10 +277,7 @@ static int imr_debugfs_register(struct imr_device *idev)
 {
 	idev->file = debugfs_create_file("imr_state", S_IFREG | S_IRUGO, NULL,
 					 idev, &imr_state_ops);
-	if (IS_ERR(idev->file))
-		return PTR_ERR(idev->file);
-
-	return 0;
+	return PTR_ERR_OR_ZERO(idev->file);
 }
 
 /**

[PATCH] x86/intel/quark: fix simple_return.cocci warnings

[kbuild test robot]

arch/x86/platform/intel-quark/imr.c:129:1-4: WARNING: end returns can be simpified

 Simplify a trivial if-return sequence.  Possibly combine with a
 preceding function call.
Generated by: scripts/coccinelle/misc/simple_return.cocci

CC: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

 imr.c |    6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

--- a/arch/x86/platform/intel-quark/imr.c
+++ b/arch/x86/platform/intel-quark/imr.c
@@ -126,12 +126,8 @@ static int imr_read(struct imr_device *i
 	if (ret)
 		return ret;
 
-	ret = iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
+	return iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
 				reg++, &imr->wmask);
-	if (ret)
-		return ret;
-
-	return 0;
 }
 
 /**

Re: [PATCH] x86/intel/quark: fix simple_return.cocci warnings

[Bryan O'Donoghue]

On 19/02/15 08:14, kbuild test robot wrote:
> arch/x86/platform/intel-quark/imr.c:129:1-4: WARNING: end returns can be simpified
>
>   Simplify a trivial if-return sequence.  Possibly combine with a
>   preceding function call.
> Generated by: scripts/coccinelle/misc/simple_return.cocci
>
> CC: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
>
>   imr.c |    6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
>
> --- a/arch/x86/platform/intel-quark/imr.c
> +++ b/arch/x86/platform/intel-quark/imr.c
> @@ -126,12 +126,8 @@ static int imr_read(struct imr_device *i
>   	if (ret)
>   		return ret;
>
> -	ret = iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
> +	return iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
>   				reg++, &imr->wmask);
> -	if (ret)
> -		return ret;
> -
> -	return 0;
>   }
>
>   /**
>

This flow was a change asked for and supplied in review feedback for 
Andy Shevchenko so NAK to this patch.

Re: [PATCH] x86/intel/quark: fix simple_return.cocci warnings

[Ingo Molnar]

* Bryan O'Donoghue <pure.logic@nexus-software.ie> wrote:

> On 19/02/15 08:14, kbuild test robot wrote:
> >arch/x86/platform/intel-quark/imr.c:129:1-4: WARNING: end returns can be simpified
> >
> >  Simplify a trivial if-return sequence.  Possibly combine with a
> >  preceding function call.
> >Generated by: scripts/coccinelle/misc/simple_return.cocci
> >
> >CC: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> >Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> >---
> >
> >  imr.c |    6 +-----
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> >--- a/arch/x86/platform/intel-quark/imr.c
> >+++ b/arch/x86/platform/intel-quark/imr.c
> >@@ -126,12 +126,8 @@ static int imr_read(struct imr_device *i
> >  	if (ret)
> >  		return ret;
> >
> >-	ret = iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
> >+	return iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
> >  				reg++, &imr->wmask);
> >-	if (ret)
> >-		return ret;
> >-
> >-	return 0;
> >  }
> >
> >  /**
> >
> 
> This flow was a change asked for and supplied in review 
> feedback for Andy Shevchenko so NAK to this patch.

But this pattern:

	if (ret)
		return ret;

	return 0;

makes very little sense. Why is it done?

Thanks,

	Ingo

Re: [PATCH] x86/intel/quark: fix simple_return.cocci warnings

[Bryan O'Donoghue]

On 19/02/15 10:25, Ingo Molnar wrote:

>>> -	ret = iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
>>> +	return iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
>>>   				reg++, &imr->wmask);
>>> -	if (ret)
>>> -		return ret;
>>> -
>>> -	return 0;
>>>   }
>>>
>>>   /**
>>>
>>
>> This flow was a change asked for and supplied in review
>> feedback for Andy Shevchenko so NAK to this patch.
>
> But this pattern:
>
> 	if (ret)
> 		return ret;
>
> 	return 0;
>
> makes very little sense. Why is it done?
>
> Thanks,
>
> 	Ingo

Feedback at review was that it's more consistent with the code that 
comes before.

So I changed it from

return iosf_mbi_read

to

ret = iosf_mbi_read

if (ret)
	return ret;
return 0;

as a result. I'm OK with the change suggested by the script if Andy (who 
requested the change) is.

--
Bryan

Re: [PATCH] x86/intel/quark: fix simple_return.cocci warnings

[Ingo Molnar]

* Bryan O'Donoghue <pure.logic@nexus-software.ie> wrote:

> On 19/02/15 10:25, Ingo Molnar wrote:
> 
> >>>-	ret = iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
> >>>+	return iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
> >>>  				reg++, &imr->wmask);
> >>>-	if (ret)
> >>>-		return ret;
> >>>-
> >>>-	return 0;
> >>>  }
> >>>
> >>>  /**
> >>>
> >>
> >>This flow was a change asked for and supplied in review
> >>feedback for Andy Shevchenko so NAK to this patch.
> >
> >But this pattern:
> >
> >	if (ret)
> >		return ret;
> >
> >	return 0;
> >
> >makes very little sense. Why is it done?
> >
> >Thanks,
> >
> >	Ingo
> 
> Feedback at review was that it's more consistent with the 
> code that comes before.

But that feedback makes very little sense. In C we don't 
ever want to write:

	if (ret)
		return ret;

	return 0;

Because we can return the fine value straight away:

	return ret;

regardless of what comes before.

Thanks,

	Ingo

Re: [PATCH] x86/intel/quark: fix simple_return.cocci warnings

[Bryan O'Donoghue]

On 19/02/15 10:33, Ingo Molnar wrote:

>> Feedback at review was that it's more consistent with the
>> code that comes before.
>
> But that feedback makes very little sense. In C we don't
> ever want to write:
>
> 	if (ret)
> 		return ret;
>
> 	return 0;
>
> Because we can return the fine value straight away:
>
> 	return ret;
>
> regardless of what comes before.

:)

Just trying to accommodate as much feedback from people as possible.

I'm fine with the change and I'll let Andy argue the counter-point if he 
wants to.

--
Bryan

[tip:x86/platform] x86/intel/quark: Fix simple_return.cocci warnings

[tip-bot for Fengguang Wu]

Commit-ID:  c11a25f443e9bee06fe302b6a78ff44dac554036
Gitweb:     http://git.kernel.org/tip/c11a25f443e9bee06fe302b6a78ff44dac554036
Author:     Fengguang Wu <fengguang.wu@intel.com>
AuthorDate: Thu, 19 Feb 2015 16:14:32 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 19 Feb 2015 10:00:55 +0100

x86/intel/quark: Fix simple_return.cocci warnings

arch/x86/platform/intel-quark/imr.c:129:1-4: WARNING: end returns can be simpified

 Simplify a trivial if-return sequence.  Possibly combine with a preceding function call.

Generated by: scripts/coccinelle/misc/simple_return.cocci

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
Cc: Andy Shevchenko <andy.schevchenko@gmail.com>
Cc: Ong, Boon Leong <boon.leong.ong@intel.com>
Cc: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Darren Hart <dvhart@linux.intel.com>
Cc: kbuild-all@01.org
Link: http://lkml.kernel.org/r/20150219081432.GA21996@waimea
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/platform/intel-quark/imr.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/x86/platform/intel-quark/imr.c b/arch/x86/platform/intel-quark/imr.c
index 60c01eb..0ee619f 100644
--- a/arch/x86/platform/intel-quark/imr.c
+++ b/arch/x86/platform/intel-quark/imr.c
@@ -126,12 +126,8 @@ static int imr_read(struct imr_device *idev, u32 imr_id, struct imr_regs *imr)
 	if (ret)
 		return ret;
 
-	ret = iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
+	return iosf_mbi_read(QRK_MBI_UNIT_MM, QRK_MBI_MM_READ,
 				reg++, &imr->wmask);
-	if (ret)
-		return ret;
-
-	return 0;
 }
 
 /**