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 <[email protected]>
Signed-off-by: Fengguang Wu <[email protected]>
---
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;
}
/**
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 <[email protected]>
> Signed-off-by: Fengguang Wu <[email protected]>
> ---
>
> 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.
* Bryan O'Donoghue <[email protected]> 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 <[email protected]>
> >Signed-off-by: Fengguang Wu <[email protected]>
> >---
> >
> > 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
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
* Bryan O'Donoghue <[email protected]> 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
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
Commit-ID: c11a25f443e9bee06fe302b6a78ff44dac554036
Gitweb: http://git.kernel.org/tip/c11a25f443e9bee06fe302b6a78ff44dac554036
Author: Fengguang Wu <[email protected]>
AuthorDate: Thu, 19 Feb 2015 16:14:32 +0800
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Ong, Boon Leong <[email protected]>
Cc: Bryan O'Donoghue <[email protected]>
Cc: Darren Hart <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/20150219081432.GA21996@waimea
Signed-off-by: Ingo Molnar <[email protected]>
---
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;
}
/**