[ovs-dev] checkpatch: Check FOR_EACH loops with numbers.
diff mbox series

Message ID 20190712130438.7431-1-i.maximets@samsung.com
State Accepted
Headers show
Series
  • [ovs-dev] checkpatch: Check FOR_EACH loops with numbers.
Related show

Commit Message

Ilya Maximets July 12, 2019, 1:04 p.m. UTC
OVS has defines for loops like 'BITMAP_FOR_EACH_1' or
'ULLONG_FOR_EACH_1', but the regexp in checkpatch doesn't match with
numbers and skips these loops while checking.

This patch adds numbers into regexp and adds some FOR_EACH loops to
the unit tests.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 tests/checkpatch.at     | 2 +-
 utilities/checkpatch.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Ben Pfaff July 12, 2019, 3:20 p.m. UTC | #1
On Fri, Jul 12, 2019 at 04:04:38PM +0300, Ilya Maximets wrote:
> OVS has defines for loops like 'BITMAP_FOR_EACH_1' or
> 'ULLONG_FOR_EACH_1', but the regexp in checkpatch doesn't match with
> numbers and skips these loops while checking.
> 
> This patch adds numbers into regexp and adds some FOR_EACH loops to
> the unit tests.
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>

Acked-by: Ben Pfaff <blp@ovn.org>
Aaron Conole July 12, 2019, 3:21 p.m. UTC | #2
Ilya Maximets <i.maximets@samsung.com> writes:

> OVS has defines for loops like 'BITMAP_FOR_EACH_1' or
> 'ULLONG_FOR_EACH_1', but the regexp in checkpatch doesn't match with
> numbers and skips these loops while checking.
>
> This patch adds numbers into regexp and adds some FOR_EACH loops to
> the unit tests.
>
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>
Ilya Maximets July 12, 2019, 3:33 p.m. UTC | #3
On 12.07.2019 18:20, Ben Pfaff wrote:
> On Fri, Jul 12, 2019 at 04:04:38PM +0300, Ilya Maximets wrote:
>> OVS has defines for loops like 'BITMAP_FOR_EACH_1' or
>> 'ULLONG_FOR_EACH_1', but the regexp in checkpatch doesn't match with
>> numbers and skips these loops while checking.
>>
>> This patch adds numbers into regexp and adds some FOR_EACH loops to
>> the unit tests.
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> 
> Acked-by: Ben Pfaff <blp@ovn.org>

Thanks, Ben.

Can we consider this as a "bug fix" and apply or should we wait until
"freeze" finished?

For me either solution is OK.

Best regards, Ilya Maximets.
Ben Pfaff July 12, 2019, 3:49 p.m. UTC | #4
On Fri, Jul 12, 2019 at 06:33:32PM +0300, Ilya Maximets wrote:
> On 12.07.2019 18:20, Ben Pfaff wrote:
> > On Fri, Jul 12, 2019 at 04:04:38PM +0300, Ilya Maximets wrote:
> >> OVS has defines for loops like 'BITMAP_FOR_EACH_1' or
> >> 'ULLONG_FOR_EACH_1', but the regexp in checkpatch doesn't match with
> >> numbers and skips these loops while checking.
> >>
> >> This patch adds numbers into regexp and adds some FOR_EACH loops to
> >> the unit tests.
> >>
> >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> > 
> > Acked-by: Ben Pfaff <blp@ovn.org>
> 
> Thanks, Ben.
> 
> Can we consider this as a "bug fix" and apply or should we wait until
> "freeze" finished?

I'd go ahead with it.  checkpatch can't break any real code.
Ilya Maximets July 12, 2019, 4:34 p.m. UTC | #5
On 12.07.2019 18:49, Ben Pfaff wrote:
> On Fri, Jul 12, 2019 at 06:33:32PM +0300, Ilya Maximets wrote:
>> On 12.07.2019 18:20, Ben Pfaff wrote:
>>> On Fri, Jul 12, 2019 at 04:04:38PM +0300, Ilya Maximets wrote:
>>>> OVS has defines for loops like 'BITMAP_FOR_EACH_1' or
>>>> 'ULLONG_FOR_EACH_1', but the regexp in checkpatch doesn't match with
>>>> numbers and skips these loops while checking.
>>>>
>>>> This patch adds numbers into regexp and adds some FOR_EACH loops to
>>>> the unit tests.
>>>>
>>>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>>>
>>> Acked-by: Ben Pfaff <blp@ovn.org>
>>
>> Thanks, Ben.
>>
>> Can we consider this as a "bug fix" and apply or should we wait until
>> "freeze" finished?
> 
> I'd go ahead with it.  checkpatch can't break any real code.

Ok.

Thanks, Ben and Aaron! Applied to master.

Best regards, Ilya Maximets.

Patch
diff mbox series

diff --git a/tests/checkpatch.at b/tests/checkpatch.at
index 07f4b137c..f3b26dd34 100755
--- a/tests/checkpatch.at
+++ b/tests/checkpatch.at
@@ -179,7 +179,7 @@  m4_define([COMMON_PATCH_HEADER], [dnl
 
 
 AT_SETUP([checkpatch - parenthesized constructs])
-for ctr in 'if' 'while' 'switch'; do
+for ctr in 'if' 'while' 'switch' 'HMAP_FOR_EACH' 'BITMAP_FOR_EACH_1'; do
 try_checkpatch \
    "COMMON_PATCH_HEADER
     +     $ctr (first_run) {
diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index ae86937c8..de5061406 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -143,7 +143,7 @@  def reset_counters():
 # something in parentheses (usually an expression) then a left curly brace.
 #
 # 'do' almost qualifies but it's also used as "do { ... } while (...);".
-__parenthesized_constructs = 'if|for|while|switch|[_A-Z]+FOR_*EACH[_A-Z]*'
+__parenthesized_constructs = 'if|for|while|switch|[_A-Z]+FOR_*EACH[_A-Z0-9]*'
 
 __regex_added_line = re.compile(r'^\+{1,2}[^\+][\w\W]*')
 __regex_subtracted_line = re.compile(r'^\-{1,2}[^\-][\w\W]*')