2014-07-08 17:23:12

by Fabian Frédérick

Subject: [PATCH 1/1] dvb-frontends: remove unnecessary break after goto

Cc: Antti Palosaari <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Cc: [email protected]
Signed-off-by: Fabian Frederick <[email protected]>
---
drivers/media/dvb-frontends/af9013.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c
index fb504f1..ecf6388 100644
--- a/drivers/media/dvb-frontends/af9013.c
+++ b/drivers/media/dvb-frontends/af9013.c
@@ -470,7 +470,6 @@ static int af9013_statistics_snr_result(struct dvb_frontend *fe)
break;
default:
goto err;
- break;
}

for (i = 0; i < len; i++) {
--
1.8.4.5

2014-07-08 18:36:04

by Antti Palosaari

Subject: Re: [PATCH 1/1] dvb-frontends: remove unnecessary break after goto

Moikka Fabian!
I have no reason to decline that patch (I will apply it) even it has
hardly meaning. But is there now some new tool which warns that kind of
issues?

regards
Atnti


On 07/08/2014 08:23 PM, Fabian Frederick wrote:
> Cc: Antti Palosaari <[email protected]>
> Cc: Mauro Carvalho Chehab <[email protected]>
> Cc: [email protected]
> Signed-off-by: Fabian Frederick <[email protected]>
> ---
> drivers/media/dvb-frontends/af9013.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c
> index fb504f1..ecf6388 100644
> --- a/drivers/media/dvb-frontends/af9013.c
> +++ b/drivers/media/dvb-frontends/af9013.c
> @@ -470,7 +470,6 @@ static int af9013_statistics_snr_result(struct dvb_frontend *fe)
> break;
> default:
> goto err;
> - break;
> }
>
> for (i = 0; i < len; i++) {
>

--
http://palosaari.fi/

2014-07-08 19:14:34

by Fabian Frédérick

Subject: Re: [PATCH 1/1] dvb-frontends: remove unnecessary break after goto

On Tue, 08 Jul 2014 21:35:58 +0300
Antti Palosaari <[email protected]> wrote:

> Moikka Fabian!
> I have no reason to decline that patch (I will apply it) even it has
> hardly meaning. But is there now some new tool which warns that kind of
> issues?
Hello Antti,

Thanks :) AFAIK there's still no automatic detection of those cases.

Regards,
Fabian
>
> regards
> Atnti
>
>
> On 07/08/2014 08:23 PM, Fabian Frederick wrote:
> > Cc: Antti Palosaari <[email protected]>
> > Cc: Mauro Carvalho Chehab <[email protected]>
> > Cc: [email protected]
> > Signed-off-by: Fabian Frederick <[email protected]>
> > ---
> > drivers/media/dvb-frontends/af9013.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/drivers/media/dvb-frontends/af9013.c b/drivers/media/dvb-frontends/af9013.c
> > index fb504f1..ecf6388 100644
> > --- a/drivers/media/dvb-frontends/af9013.c
> > +++ b/drivers/media/dvb-frontends/af9013.c
> > @@ -470,7 +470,6 @@ static int af9013_statistics_snr_result(struct dvb_frontend *fe)
> > break;
> > default:
> > goto err;
> > - break;
> > }
> >
> > for (i = 0; i < len; i++) {
> >
>
> --
> http://palosaari.fi/

2014-07-08 19:57:47

by Joe Perches

Subject: [PATCH] checkpatch: Warn on break after goto or return with same tab indentation

Using break; after a goto or return is unnecessary so
emit a warning when the break is at the same indent level.

So this emits a warning on:

switch (foo) {
case 1:
goto err;
break;
}

but not on:

switch (foo) {
case 1:
if (bar())
goto err;
break;
}

Signed-off-by: Joe Perches <[email protected]>
---
> AFAIK there's still no automatic detection of those cases.

There can be now...

scripts/checkpatch.pl | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 496f9ab..fc22f22 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2439,6 +2439,16 @@ sub process {
}
}

+# check indentation of a line with a break;
+# if the previous line is a goto or return and is indented the same # of tabs
+ if ($sline =~ /^\+([\t]+)break\s*;\s*$/) {
+ my $tabs = $1;
+ if ($prevline =~ /^\+$tabs(?:goto|return)\b/) {
+ WARN("UNNECESSARY_BREAK",
+ "break is not useful after a goto or return\n" . $hereprev);
+ }
+ }
+
# discourage the addition of CONFIG_EXPERIMENTAL in #if(def).
if ($line =~ /^\+\s*\#\s*if.*\bCONFIG_EXPERIMENTAL\b/) {
WARN("CONFIG_EXPERIMENTAL",