diff mbox

[ovs-dev] checkpatch: Check for trailing operators.

Message ID 20170714205906.13268-1-joe@ovn.org
State Changes Requested
Headers show

Commit Message

Joe Stringer July 14, 2017, 8:59 p.m. UTC
The style guide states that lines should not end with '&&', '||' '&'
'|', '?', ':'. Check for this and report an error.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
 utilities/checkpatch.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Ben Pfaff July 14, 2017, 9:03 p.m. UTC | #1
On Fri, Jul 14, 2017 at 01:59:06PM -0700, Joe Stringer wrote:
> The style guide states that lines should not end with '&&', '||' '&'
> '|', '?', ':'. Check for this and report an error.
> 
> Signed-off-by: Joe Stringer <joe@ovn.org>

Hmm, is that right?  It's pretty explicit about ?:, but I don't know
about the others.
Joe Stringer July 14, 2017, 9:22 p.m. UTC | #2
On 14 July 2017 at 14:03, Ben Pfaff <blp@ovn.org> wrote:
> On Fri, Jul 14, 2017 at 01:59:06PM -0700, Joe Stringer wrote:
>> The style guide states that lines should not end with '&&', '||' '&'
>> '|', '?', ':'. Check for this and report an error.
>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>
> Hmm, is that right?  It's pretty explicit about ?:, but I don't know
> about the others.

Ah, right you are. It's not explicit about the others. That said,
following the explicit description for ?: there's a few examples that
apply this style also to &&,||,&,|. I can respin this patch if you
think this is too strict.
Ben Pfaff July 14, 2017, 9:29 p.m. UTC | #3
On Fri, Jul 14, 2017 at 02:22:16PM -0700, Joe Stringer wrote:
> On 14 July 2017 at 14:03, Ben Pfaff <blp@ovn.org> wrote:
> > On Fri, Jul 14, 2017 at 01:59:06PM -0700, Joe Stringer wrote:
> >> The style guide states that lines should not end with '&&', '||' '&'
> >> '|', '?', ':'. Check for this and report an error.
> >>
> >> Signed-off-by: Joe Stringer <joe@ovn.org>
> >
> > Hmm, is that right?  It's pretty explicit about ?:, but I don't know
> > about the others.
> 
> Ah, right you are. It's not explicit about the others. That said,
> following the explicit description for ?: there's a few examples that
> apply this style also to &&,||,&,|. I can respin this patch if you
> think this is too strict.

I personally like to treat && || & | on a case-by-case basis.  Sometimes
stuff lines up nicer one way, sometimes the other.
Joe Stringer July 14, 2017, 9:51 p.m. UTC | #4
On 14 July 2017 at 14:29, Ben Pfaff <blp@ovn.org> wrote:
> On Fri, Jul 14, 2017 at 02:22:16PM -0700, Joe Stringer wrote:
>> On 14 July 2017 at 14:03, Ben Pfaff <blp@ovn.org> wrote:
>> > On Fri, Jul 14, 2017 at 01:59:06PM -0700, Joe Stringer wrote:
>> >> The style guide states that lines should not end with '&&', '||' '&'
>> >> '|', '?', ':'. Check for this and report an error.
>> >>
>> >> Signed-off-by: Joe Stringer <joe@ovn.org>
>> >
>> > Hmm, is that right?  It's pretty explicit about ?:, but I don't know
>> > about the others.
>>
>> Ah, right you are. It's not explicit about the others. That said,
>> following the explicit description for ?: there's a few examples that
>> apply this style also to &&,||,&,|. I can respin this patch if you
>> think this is too strict.
>
> I personally like to treat && || & | on a case-by-case basis.  Sometimes
> stuff lines up nicer one way, sometimes the other.

OK.
diff mbox

Patch

diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 65d188d607f1..543762c22bd9 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -87,6 +87,7 @@  __regex_ends_with_bracket = \
     re.compile(r'[^\s]\) {(\s+/\*[\s\Sa-zA-Z0-9\.,\?\*/+-]*)?$')
 __regex_ptr_declaration_missing_whitespace = re.compile(r'[a-zA-Z0-9]\*[^*]')
 __regex_is_comment_line = re.compile(r'^\s*(/\*|\*\s)')
+__regex_trailing_operator = re.compile(r'.*[&|]?[?:&|]$')
 
 skip_leading_whitespace_check = False
 skip_trailing_whitespace_check = False
@@ -199,6 +200,11 @@  def is_comment_line(line):
     return __regex_is_comment_line.match(line) is not None
 
 
+def trailing_operator(line):
+    """Returns TRUE if the current line ends with an operator, eg &&"""
+    return __regex_trailing_operator.match(line) is not None
+
+
 checks = [
     {'regex': None,
      'match_name':
@@ -230,7 +236,12 @@  checks = [
      'prereq': lambda x: not is_comment_line(x),
      'check': lambda x: pointer_whitespace_check(x),
      'print':
-     lambda: print_error("Inappropriate spacing in pointer declaration")}
+     lambda: print_error("Inappropriate spacing in pointer declaration")},
+
+    {'regex': '(.c|.h)(.in)?$', 'match_name': None,
+     'prereq': lambda x: not is_comment_line(x),
+     'check': lambda x: trailing_operator(x),
+     'print': lambda: print_error("Line has operator at end of line")},
 ]