* [PATCH] checkpatch: Make "return is not a function" test quieter [not found] ` <20140227184723.GA3905@kroah.com> @ 2014-02-27 19:27 ` Joe Perches 2014-02-27 22:44 ` [OPW kernel] " josh 0 siblings, 1 reply; 2+ messages in thread From: Joe Perches @ 2014-02-27 19:27 UTC (permalink / raw) To: Andrew Morton Cc: Julia Lawall, Josh Triplett, Monam Agarwal, opw-kernel, Greg KH, LKML This test is a bit noisy and opinions seem to agree that it should not warn in a lot more situations. It seems people agree that: return (foo || bar); and return foo || bar; are both acceptable style and checkpatch should be silent about them. For now, it warns on parentheses around a simple constant or a single function or a ternary. return (foo); return (foo(bar)); return (foo ? bar : baz); The last ternary test may be quieted in the future. Modify the deparenthesize function to only strip balanced leading and trailing parentheses. Signed-off-by: Joe Perches <joe@perches.com> --- scripts/checkpatch.pl | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 91308be..be4be81 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -439,9 +439,14 @@ our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)}; sub deparenthesize { my ($string) = @_; return "" if (!defined($string)); - $string =~ s@^\s*\(\s*@@g; - $string =~ s@\s*\)\s*$@@g; + + while ($string =~ /^\s*\(.*\)\s*$/) { + $string =~ s@^\s*\(\s*@@; + $string =~ s@\s*\)\s*$@@; + } + $string =~ s@\s+@ @g; + return $string; } @@ -3362,14 +3367,17 @@ sub process { } } -# Return is not a function. +# return is not a function if (defined($stat) && $stat =~ /^.\s*return(\s*)\(/s) { my $spacing = $1; if ($^V && $^V ge 5.10.0 && - $stat =~ /^.\s*return\s*$balanced_parens\s*;\s*$/) { - ERROR("RETURN_PARENTHESES", - "return is not a function, parentheses are not required\n" . $herecurr); - + $stat =~ /^.\s*return\s*($balanced_parens)\s*;\s*$/) { + my $value = $1; + $value = deparenthesize($value); + if ($value =~ m/^\s*$FuncArg\s*(?:\?|$)/) { + ERROR("RETURN_PARENTHESES", + "return is not a function, parentheses are not required\n" . $herecurr); + } } elsif ($spacing !~ /\s+/) { ERROR("SPACING", "space required before the open parenthesis '('\n" . $herecurr); ^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [OPW kernel] [PATCH] checkpatch: Make "return is not a function" test quieter 2014-02-27 19:27 ` [PATCH] checkpatch: Make "return is not a function" test quieter Joe Perches @ 2014-02-27 22:44 ` josh 0 siblings, 0 replies; 2+ messages in thread From: josh @ 2014-02-27 22:44 UTC (permalink / raw) To: Joe Perches Cc: Andrew Morton, Julia Lawall, Monam Agarwal, opw-kernel, Greg KH, LKML On Thu, Feb 27, 2014 at 11:27:10AM -0800, Joe Perches wrote: > This test is a bit noisy and opinions seem to agree that > it should not warn in a lot more situations. > > It seems people agree that: > > return (foo || bar); > and > return foo || bar; > > are both acceptable style and checkpatch should be silent > about them. > > For now, it warns on parentheses around a simple constant > or a single function or a ternary. > > return (foo); > return (foo(bar)); > return (foo ? bar : baz); > > The last ternary test may be quieted in the future. > > Modify the deparenthesize function to only strip balanced > leading and trailing parentheses. > > Signed-off-by: Joe Perches <joe@perches.com> I'd suggest dropping the warning for parenthesized ternaries as well, but in any case: Reviewed-by: Josh Triplett <josh@joshtriplett.org> > scripts/checkpatch.pl | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 91308be..be4be81 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -439,9 +439,14 @@ our $FuncArg = qr{$Typecast{0,1}($LvalOrFunc|$Constant)}; > sub deparenthesize { > my ($string) = @_; > return "" if (!defined($string)); > - $string =~ s@^\s*\(\s*@@g; > - $string =~ s@\s*\)\s*$@@g; > + > + while ($string =~ /^\s*\(.*\)\s*$/) { > + $string =~ s@^\s*\(\s*@@; > + $string =~ s@\s*\)\s*$@@; > + } > + > $string =~ s@\s+@ @g; > + > return $string; > } > > @@ -3362,14 +3367,17 @@ sub process { > } > } > > -# Return is not a function. > +# return is not a function > if (defined($stat) && $stat =~ /^.\s*return(\s*)\(/s) { > my $spacing = $1; > if ($^V && $^V ge 5.10.0 && > - $stat =~ /^.\s*return\s*$balanced_parens\s*;\s*$/) { > - ERROR("RETURN_PARENTHESES", > - "return is not a function, parentheses are not required\n" . $herecurr); > - > + $stat =~ /^.\s*return\s*($balanced_parens)\s*;\s*$/) { > + my $value = $1; > + $value = deparenthesize($value); > + if ($value =~ m/^\s*$FuncArg\s*(?:\?|$)/) { > + ERROR("RETURN_PARENTHESES", > + "return is not a function, parentheses are not required\n" . $herecurr); > + } > } elsif ($spacing !~ /\s+/) { > ERROR("SPACING", > "space required before the open parenthesis '('\n" . $herecurr); > > > -- > You received this message because you are subscribed to the Google Groups "opw-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to opw-kernel+unsubscribe@googlegroups.com. > For more options, visit https://groups.google.com/groups/opt_out. ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-02-27 22:44 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <alpine.DEB.2.10.1402261908070.2159@hadrien>
[not found] ` <20140226182832.GA23053@cloud>
[not found] ` <20140227164152.GA26266@kroah.com>
[not found] ` <20140227170811.GA28796@thin>
[not found] ` <alpine.DEB.2.10.1402271811010.4439@hadrien>
[not found] ` <20140227172017.GA29133@thin>
[not found] ` <alpine.DEB.2.10.1402271822540.4439@hadrien>
[not found] ` <1393523563.24588.101.camel@joe-AO722>
[not found] ` <alpine.DEB.2.10.1402271856530.4439@hadrien>
[not found] ` <1393524256.24588.103.camel@joe-AO722>
[not found] ` <20140227184723.GA3905@kroah.com>
2014-02-27 19:27 ` [PATCH] checkpatch: Make "return is not a function" test quieter Joe Perches
2014-02-27 22:44 ` [OPW kernel] " josh
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox