Messages in this thread | | | Subject | Re: [RFC PATCH 1/1] checkpatch: don't suggest else is not useful with chained | From | Joe Perches <> | Date | Mon, 21 Mar 2022 07:24:53 -0700 |
| |
On Mon, 2022-03-21 at 11:18 +0200, Ilpo Järvinen wrote: > When if blocks are chained with "else if" like this: > > if (a) > c = 0; > else if (b) > break; > else > c = 1; > > checkpatch recommends removing else: > > WARNING: else is not generally useful after a break or return > > Removing else will easily introduce logic errors in this situation > so it's better to check if the preceding line has "else if" before > issuing that warning. > > Fixes: 032a4c0f9a77 ("checkpatch: warn on unnecessary else after return or break") > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> > --- > > I was only able to solve this case for single line if blocks but > it would be useful to address also multi-line if blocks but it is > beyond my understanding about checkpatch internals. > > scripts/checkpatch.pl | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl [] > @@ -4037,14 +4037,17 @@ sub process { > # check indentation of any line with a bare else > # (but not if it is a multiple line "if (foo) return bar; else return baz;") > # if the previous line is a break or return and is indented 1 tab more... > +# but don't warn when there is "else if" on before that line to avoid logic errors > if ($sline =~ /^\+([\t]+)(?:}[ \t]*)?else(?:[ \t]*{)?\s*$/) { > my $tabs = length($1) + 1; > - if ($prevline =~ /^\+\t{$tabs,$tabs}break\b/ || > - ($prevline =~ /^\+\t{$tabs,$tabs}return\b/ && > - defined $lines[$linenr] && > - $lines[$linenr] !~ /^[ \+]\t{$tabs,$tabs}return/)) { > + if ($linenr >= 3 && > + $lines[$linenr - 3] !~ /[} \t]else\s+if\s*\(/ && > + ($prevline =~ /^\+\t{$tabs,$tabs}break\b/ || > + ($prevline =~ /^\+\t{$tabs,$tabs}return\b/ && > + defined $lines[$linenr] && > + $lines[$linenr] !~ /^[ \+]\t{$tabs,$tabs}return/))) { > WARN("UNNECESSARY_ELSE", > - "else is not generally useful after a break or return\n" . $hereprev); > + "else is not generally useful after a break or return\n" . $hereprev . "\n");
I believe this wouldn't work on patches if the lines above are deletions.
| |