diff mbox

[ovs-dev] checkpatch: Enforce bracing around conditionals.

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

Commit Message

Joe Stringer Aug. 16, 2017, 11:01 p.m. UTC
The coding style states that BSD-style brace placement should be used,
and even single statements should be enclosed. Add checks to checkpatch
for this.

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

Comments

Aaron Conole Aug. 17, 2017, 2:35 p.m. UTC | #1
Hi Joe,

Joe Stringer <joe@ovn.org> writes:

> The coding style states that BSD-style brace placement should be used,
> and even single statements should be enclosed. Add checks to checkpatch
> for this.
>
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
>  utilities/checkpatch.py | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index 43f10bb3ded3..c8381c98403b 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -95,6 +95,8 @@ __regex_ends_with_bracket = \
>  __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'^[^ ]* [^ ]*[?:]$')
> +__regex_conditional_else_bracing = re.compile(r'^\s*else\s*{?$')
> +__regex_conditional_else_bracing2 = re.compile(r'^\s*}\selse\s*$')
>  
>  skip_leading_whitespace_check = False
>  skip_trailing_whitespace_check = False
> @@ -212,6 +214,12 @@ def trailing_operator(line):
>      return __regex_trailing_operator.match(line) is not None
>  
>  
> +def if_else_bracing_check(line):
> +    if __regex_conditional_else_bracing.match(line) is not None:
> +        return True
> +    return __regex_conditional_else_bracing2.match(line) is not None
> +
> +

Would it make more sense to put this in with the other bracing checks in
if_and_for_end_with_bracket_check ?

>  checks = [
>      {'regex': None,
>       'match_name':
> @@ -250,6 +258,11 @@ checks = [
>       'check': lambda x: trailing_operator(x),
>       'print':
>       lambda: print_error("Line has '?' or ':' operator at end of line")},
> +
> +    {'regex': '(.c|.h)(.in)?$', 'match_name': None,
> +     'prereq': lambda x: not is_comment_line(x),
> +     'check': lambda x: if_else_bracing_check(x),
> +     'print': lambda: print_error("Improper bracing in conditional statement")}
>  ]
Joe Stringer Aug. 17, 2017, 9:24 p.m. UTC | #2
On 17 August 2017 at 07:35, Aaron Conole <aconole@redhat.com> wrote:
> Hi Joe,
>
> Joe Stringer <joe@ovn.org> writes:
>
>> The coding style states that BSD-style brace placement should be used,
>> and even single statements should be enclosed. Add checks to checkpatch
>> for this.
>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>> ---
>>  utilities/checkpatch.py | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
>> index 43f10bb3ded3..c8381c98403b 100755
>> --- a/utilities/checkpatch.py
>> +++ b/utilities/checkpatch.py
>> @@ -95,6 +95,8 @@ __regex_ends_with_bracket = \
>>  __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'^[^ ]* [^ ]*[?:]$')
>> +__regex_conditional_else_bracing = re.compile(r'^\s*else\s*{?$')
>> +__regex_conditional_else_bracing2 = re.compile(r'^\s*}\selse\s*$')
>>
>>  skip_leading_whitespace_check = False
>>  skip_trailing_whitespace_check = False
>> @@ -212,6 +214,12 @@ def trailing_operator(line):
>>      return __regex_trailing_operator.match(line) is not None
>>
>>
>> +def if_else_bracing_check(line):
>> +    if __regex_conditional_else_bracing.match(line) is not None:
>> +        return True
>> +    return __regex_conditional_else_bracing2.match(line) is not None
>> +
>> +
>
> Would it make more sense to put this in with the other bracing checks in
> if_and_for_end_with_bracket_check ?

Yes! Apparently I missed this before. Thanks, I'll respin.
diff mbox

Patch

diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 43f10bb3ded3..c8381c98403b 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -95,6 +95,8 @@  __regex_ends_with_bracket = \
 __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'^[^ ]* [^ ]*[?:]$')
+__regex_conditional_else_bracing = re.compile(r'^\s*else\s*{?$')
+__regex_conditional_else_bracing2 = re.compile(r'^\s*}\selse\s*$')
 
 skip_leading_whitespace_check = False
 skip_trailing_whitespace_check = False
@@ -212,6 +214,12 @@  def trailing_operator(line):
     return __regex_trailing_operator.match(line) is not None
 
 
+def if_else_bracing_check(line):
+    if __regex_conditional_else_bracing.match(line) is not None:
+        return True
+    return __regex_conditional_else_bracing2.match(line) is not None
+
+
 checks = [
     {'regex': None,
      'match_name':
@@ -250,6 +258,11 @@  checks = [
      'check': lambda x: trailing_operator(x),
      'print':
      lambda: print_error("Line has '?' or ':' operator at end of line")},
+
+    {'regex': '(.c|.h)(.in)?$', 'match_name': None,
+     'prereq': lambda x: not is_comment_line(x),
+     'check': lambda x: if_else_bracing_check(x),
+     'print': lambda: print_error("Improper bracing in conditional statement")}
 ]