| Message ID | 1488219268-3006-5-git-send-email-singhalsimran0@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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); }
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(-)