| Message ID | 1561116534-21814-1-git-send-email-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] checkpatch: do not warn for multiline parenthesized returned value | expand |
On 6/21/19 6:28 AM, Paolo Bonzini wrote: > While indeed we do not want to have > > return (a); > > it is less clear that this applies to > > return (a && > b); > > Some editors indent more nicely if you have parentheses, and some people's > eyes may appreciate that as well. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > scripts/checkpatch.pl | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) I'm certainly in favor of this (as I've been known to use this style). > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index c2aaf42..2f81371 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2296,7 +2296,8 @@ sub process { > $value =~ s/\([^\(\)]*\)/1/) { > } > #print "value<$value>\n"; > - if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/) { > + if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/ && > + $line =~ /;$/) { So the diagnosis now checks for a trailing ';' as its witness of whether this is a one-liner return statement, leaving multi-liners undiagnosed. Easy enough to understand. Reviewed-by: Eric Blake <eblake@redhat.com>
Paolo Bonzini <pbonzini@redhat.com> writes: > While indeed we do not want to have > > return (a); > > it is less clear that this applies to > > return (a && > b); > > Some editors indent more nicely if you have parentheses, and some people's > eyes may appreciate that as well. No objection. > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > scripts/checkpatch.pl | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index c2aaf42..2f81371 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2296,7 +2296,8 @@ sub process { > $value =~ s/\([^\(\)]*\)/1/) { > } > #print "value<$value>\n"; > - if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/) { > + if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/ && > + $line =~ /;$/) { > ERROR("return is not a function, parentheses are not required\n" . $herecurr); > > } elsif ($spacing !~ /\s+/) {
On 6/21/19 1:28 PM, Paolo Bonzini wrote: > While indeed we do not want to have > > return (a); > > it is less clear that this applies to > > return (a && > b); > > Some editors indent more nicely if you have parentheses, and some people's > eyes may appreciate that as well. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > scripts/checkpatch.pl | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Thanks, I do this semi-regularly. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c2aaf42..2f81371 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2296,7 +2296,8 @@ sub process { $value =~ s/\([^\(\)]*\)/1/) { } #print "value<$value>\n"; - if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/) { + if ($value =~ /^\s*(?:$Ident|-?$Constant)\s*$/ && + $line =~ /;$/) { ERROR("return is not a function, parentheses are not required\n" . $herecurr); } elsif ($spacing !~ /\s+/) {
While indeed we do not want to have return (a); it is less clear that this applies to return (a && b); Some editors indent more nicely if you have parentheses, and some people's eyes may appreciate that as well. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- scripts/checkpatch.pl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)