Messages in this thread | | | Date | Tue, 09 Sep 2014 17:08:21 +0200 | From | Nitin Kuppelur <> | Subject | Re: [PATCH] scripts: checkpatch.pl add warning for dummy label |
| |
Hi Joe,
Thanks for feedback.
The idea of adding new warning came from the review comments from one of my PATCH, where it was suggested by few reviewers to remove such "Do-Nothing" labels. But current checkpatch.pl does not warn about that.
[PATCH] http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg720639.html
Regards, Nitin
On 09/09/2014 05:43 PM, Joe Perches wrote: > On Tue, 2014-09-09 at 15:54 +0200, Nitin Kuppelur wrote: >> Added new warning DUMMY_LABEL in checkpatch.pl. This warns >> if return statement is encountered just after goto label target >> like "out:". In such scenario it is best to call "return;" directly >> instead of "goto out;" > > I'm not sure this is desired/correct. > > There are many functions written like: > > return_type foo(...) > { > [some initialization...] > > err = some_test(...); > if (err) > goto out; > > [...] > > out: > [some cleanup...] > return err; > } > > In many cases, a void function is a simple > variant of a non-void function and many people > like to see the same code "shape" in functions, > just without the init and cleanup bits. > >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > [] >> @@ -3798,10 +3798,14 @@ sub process { >> if ($sline =~ /^[ \+]}\s*$/ && >> $prevline =~ /^\+\treturn\s*;\s*$/ && >> $linenr >= 3 && >> - $lines[$linenr - 3] =~ /^[ +]/ && >> - $lines[$linenr - 3] !~ /^[ +]\s*$Ident\s*:/) { >> - WARN("RETURN_VOID", >> - "void function return statements are not generally useful\n" . $hereprev); >> + $lines[$linenr - 3] =~ /^[ +]/) { >> + if ($lines[$linenr - 3] !~ /^[ +]\s*$Ident\s*:/) { >> + WARN("RETURN_VOID", >> + "void function return statements are not generally useful\n" . $hereprev); >> + } elsif ($lines[$linenr - 3] =~ /^[ +]\s*$Ident\s*:/) { >> + WARN("DUMMY_LABEL", >> + "labels doing nothing are not generally useful\n" . $hereprev); >> + } >> } >> >> # if statements using unnecessary parentheses - ie: if ((foo == bar)) > > >
| |