Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1588850 > unrolled thread

[PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return simran singhal <singhalsimran0@gmail.com> - 2017-02-27 19:20 +0100
    Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after  return Joe Perches <joe@perches.com> - 2017-02-27 21:10 +0100
      Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return SIMRAN SINGHAL <singhalsimran0@gmail.com> - 2017-02-27 21:50 +0100
        Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after  return Joe Perches <joe@perches.com> - 2017-02-28 20:50 +0100
          Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove  unnecessary else after return Julia Lawall <julia.lawall@lip6.fr> - 2017-02-28 21:10 +0100
        Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove  unnecessary else after return Julia Lawall <julia.lawall@lip6.fr> - 2017-02-28 20:50 +0100
        Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return SIMRAN SINGHAL <singhalsimran0@gmail.com> - 2017-02-28 21:20 +0100

#1588850 — [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

This patch fixes the checkpatch warning that else is not generally
useful after a break or return.

This was done using Coccinelle:
@@
expression e2;
statement s1;
@@
if(e2) { ... return ...; }
-else
         s1

Signed-off-by: simran singhal <singhalsimran0@gmail.com>
---
 drivers/staging/gdm724x/gdm_endian.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
index d0b43e2..4ef728f 100644
--- a/drivers/staging/gdm724x/gdm_endian.c
+++ b/drivers/staging/gdm724x/gdm_endian.c
@@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
 {
 	if (ed->dev_ed == ENDIANNESS_LITTLE)
 		return (__force __dev16)cpu_to_le16(x);
-	else
-		return (__force __dev16)cpu_to_be16(x);
+	return (__force __dev16)cpu_to_be16(x);
 }
 
 u16 gdm_dev16_to_cpu(struct gdm_endian *ed, __dev16 x)
 {
 	if (ed->dev_ed == ENDIANNESS_LITTLE)
 		return le16_to_cpu((__force __le16)x);
-	else
-		return be16_to_cpu((__force __be16)x);
+	return be16_to_cpu((__force __be16)x);
 }
 
 __dev32 gdm_cpu_to_dev32(struct gdm_endian *ed, u32 x)
 {
 	if (ed->dev_ed == ENDIANNESS_LITTLE)
 		return (__force __dev32)cpu_to_le32(x);
-	else
-		return (__force __dev32)cpu_to_be32(x);
+	return (__force __dev32)cpu_to_be32(x);
 }
 
 u32 gdm_dev32_to_cpu(struct gdm_endian *ed, __dev32 x)
 {
 	if (ed->dev_ed == ENDIANNESS_LITTLE)
 		return le32_to_cpu((__force __le32)x);
-	else
-		return be32_to_cpu((__force __be32)x);
+	return be32_to_cpu((__force __be32)x);
 }
-- 
2.7.4

[toc] | [next] | [standalone]

#1588907 — Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> This patch fixes the checkpatch warning that else is not generally
> useful after a break or return.

> This was done using Coccinelle:
> @@
> expression e2;
> statement s1;
> @@
> if(e2) { ... return ...; }
> -else
>          s1
[]
> diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
[]
> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
>  {
>  	if (ed->dev_ed == ENDIANNESS_LITTLE)
>  		return (__force __dev16)cpu_to_le16(x);
> -	else
> -		return (__force __dev16)cpu_to_be16(x);
> +	return (__force __dev16)cpu_to_be16(x);

again, not a checkpatch message for any of the
suggested modified hunks.

[toc] | [prev] | [next] | [standalone]

#1588937

On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
>> This patch fixes the checkpatch warning that else is not generally
>> useful after a break or return.
>
>> This was done using Coccinelle:
>> @@
>> expression e2;
>> statement s1;
>> @@
>> if(e2) { ... return ...; }
>> -else
>>          s1
> []
>> diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
> []
>> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
>>  {
>>       if (ed->dev_ed == ENDIANNESS_LITTLE)
>>               return (__force __dev16)cpu_to_le16(x);
>> -     else
>> -             return (__force __dev16)cpu_to_be16(x);
>> +     return (__force __dev16)cpu_to_be16(x);
>
> again, not a checkpatch message for any of the
> suggested modified hunks.
>
So, I have to change commit message.

[toc] | [prev] | [next] | [standalone]

#1589729 — Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

On Wed, 2017-03-01 at 00:41 +0530, SIMRAN SINGHAL wrote:
> On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
> <singhalsimran0@gmail.com> wrote:
> > On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
> > > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> > > > This patch fixes the checkpatch warning that else is not generally
> > > > useful after a break or return.
> > > > This was done using Coccinelle:
> > > > @@
> > > > expression e2;
> > > > statement s1;
> > > > @@
> > > > if(e2) { ... return ...; }
> > > > -else
> > > >          s1
> > > 
> > > []
> > > > diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
> > > 
> > > []
> > > > @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
> > > >  {
> > > >       if (ed->dev_ed == ENDIANNESS_LITTLE)
> > > >               return (__force __dev16)cpu_to_le16(x);
> > > > -     else
> > > > -             return (__force __dev16)cpu_to_be16(x);
> > > > +     return (__force __dev16)cpu_to_be16(x);
> > > 
> > > again, not a checkpatch message for any of the
> > > suggested modified hunks.
> > > 
> 
> I am not getting what's the problem in removing else or may be I
> am wrong you just want to say that I should change the commit message.

2 things:

1: The commit message is incorrect.
2: This form is fundamentally OK:

	if (foo)
		return bar;
	else
		return baz;


So I think this patch is not good.

[toc] | [prev] | [next] | [standalone]

#1589742 — Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

On Tue, 28 Feb 2017, Joe Perches wrote:

> On Wed, 2017-03-01 at 00:41 +0530, SIMRAN SINGHAL wrote:
> > On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
> > <singhalsimran0@gmail.com> wrote:
> > > On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
> > > > On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> > > > > This patch fixes the checkpatch warning that else is not generally
> > > > > useful after a break or return.
> > > > > This was done using Coccinelle:
> > > > > @@
> > > > > expression e2;
> > > > > statement s1;
> > > > > @@
> > > > > if(e2) { ... return ...; }
> > > > > -else
> > > > >          s1
> > > >
> > > > []
> > > > > diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
> > > >
> > > > []
> > > > > @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
> > > > >  {
> > > > >       if (ed->dev_ed == ENDIANNESS_LITTLE)
> > > > >               return (__force __dev16)cpu_to_le16(x);
> > > > > -     else
> > > > > -             return (__force __dev16)cpu_to_be16(x);
> > > > > +     return (__force __dev16)cpu_to_be16(x);
> > > >
> > > > again, not a checkpatch message for any of the
> > > > suggested modified hunks.
> > > >
> >
> > I am not getting what's the problem in removing else or may be I
> > am wrong you just want to say that I should change the commit message.
>
> 2 things:
>
> 1: The commit message is incorrect.
> 2: This form is fundamentally OK:
>
> 	if (foo)
> 		return bar;
> 	else
> 		return baz;
>
>
> So I think this patch is not good.

I agree in this case.  The two branches are quite parallel.  In some of
the other patches, the if was looking for the absence of some resource or
the failure of something, so there was a clear distinction between one
branch being cleanup on failure and the other branch being the continuing
successful computation, even if it is just to return a success indicator.

julia

[toc] | [prev] | [next] | [standalone]

#1589732 — Re: [Outreachy kernel] Re: [PATCH 5/5] staging: gdm724x: Remove unnecessary else after return

On Wed, 1 Mar 2017, SIMRAN SINGHAL wrote:

> On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
> <singhalsimran0@gmail.com> wrote:
> > On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
> >> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
> >>> This patch fixes the checkpatch warning that else is not generally
> >>> useful after a break or return.
> >>
> >>> This was done using Coccinelle:
> >>> @@
> >>> expression e2;
> >>> statement s1;
> >>> @@
> >>> if(e2) { ... return ...; }
> >>> -else
> >>>          s1
> >> []
> >>> diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
> >> []
> >>> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
> >>>  {
> >>>       if (ed->dev_ed == ENDIANNESS_LITTLE)
> >>>               return (__force __dev16)cpu_to_le16(x);
> >>> -     else
> >>> -             return (__force __dev16)cpu_to_be16(x);
> >>> +     return (__force __dev16)cpu_to_be16(x);
> >>
> >> again, not a checkpatch message for any of the
> >> suggested modified hunks.
> >>
> I am not getting what's the problem in removing else or may be I
> am wrong you just want to say that I should change the commit message.

Yes, I think that the issue is just the commit message.  Was it really
checkpatch that motivated you to do this?  Joe maintains checkpatch, and
he doesn't think that it gives such a warning.

julia

[toc] | [prev] | [next] | [standalone]

#1589746

On Tue, Feb 28, 2017 at 1:49 AM, SIMRAN SINGHAL
<singhalsimran0@gmail.com> wrote:
> On Tue, Feb 28, 2017 at 1:11 AM, Joe Perches <joe@perches.com> wrote:
>> On Mon, 2017-02-27 at 23:44 +0530, simran singhal wrote:
>>> This patch fixes the checkpatch warning that else is not generally
>>> useful after a break or return.
>>
>>> This was done using Coccinelle:
>>> @@
>>> expression e2;
>>> statement s1;
>>> @@
>>> if(e2) { ... return ...; }
>>> -else
>>>          s1
>> []
>>> diff --git a/drivers/staging/gdm724x/gdm_endian.c b/drivers/staging/gdm724x/gdm_endian.c
>> []
>>> @@ -26,30 +26,26 @@ __dev16 gdm_cpu_to_dev16(struct gdm_endian *ed, u16 x)
>>>  {
>>>       if (ed->dev_ed == ENDIANNESS_LITTLE)
>>>               return (__force __dev16)cpu_to_le16(x);
>>> -     else
>>> -             return (__force __dev16)cpu_to_be16(x);
>>> +     return (__force __dev16)cpu_to_be16(x);
>>
>> again, not a checkpatch message for any of the
>> suggested modified hunks.
>>
I am not getting what's the problem in removing else or may be I
am wrong you just want to say that I should change the commit message.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web