On 23 May 2022, at 10:56, Peng He wrote:

> Eelco Chaudron <[email protected]> 于2022年5月23日周一 16:04写道:
>
>>
>>
>> On 20 May 2022, at 12:24, Peng He wrote:
>>
>>> Signed-off-by: Peng He <[email protected]>
>>> Signed-off-by: Peng He <[email protected]>
>>> Acked-by: Eelco Chaudron <[email protected]>
>>> Acked-by: Aaron Conole <[email protected]>
>>
>> You should remove any ACKs if you re-send the patch with changes. In
>> addition only sending this patch out of the series is confusing, so you
>> need to re-send the entire series.
>>
>
> OK.
>
>>
>>> ---
>>>  tests/checkpatch.at     | 29 +++++++++++++++++++++++++++++
>>>  utilities/checkpatch.py | 36 +++++++++++++++++++++++++++++++++---
>>>  2 files changed, 62 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tests/checkpatch.at b/tests/checkpatch.at
>>> index fadb625e9..f77a55d76 100755
>>> --- a/tests/checkpatch.at
>>> +++ b/tests/checkpatch.at
>>> @@ -170,6 +170,17 @@ m4_define([COMMON_PATCH_HEADER], [dnl
>>>      +++ b/A.c
>>>      @@ -1,1 +1,1 @@])
>>>
>>> +m4_define([PATCH_HEADER], [dnl
>>> +    Author: A
>>> +
>>> +    Signed-off-by: A
>>> +    ---
>>> +    diff --git a/$1 b/$1
>>> +    index 0000000..1111111 100644
>>> +    --- a/$1
>>> +    +++ b/$1
>>> +    @@ -1,1 +1,1 @@])
>>> +
>>>
>>>  AT_SETUP([checkpatch - parenthesized constructs])
>>>  for ctr in 'if' 'while' 'switch' 'HMAP_FOR_EACH' 'BITMAP_FOR_EACH_1'; do
>>> @@ -348,6 +359,24 @@ try_checkpatch \
>>>
>>>  AT_CLEANUP
>>>
>>> +AT_SETUP([checkpatch - check misuse APIs])
>>> +try_checkpatch \
>>> +   "PATCH_HEADER([a.c])
>>
>> There is already the COMMON_PATCH_HEADER, this should work here as well.
>>
>
> you mean by expending COMMON_PATCH_HEADER, like use _COMMON_PATCH_HEADER
> with a
> parameter, the PATCH_HEADER has a file parameter.

You could just use COMMON_PATCH_HEADER with a fixed file name? Do you really 
need the file parameter, you can’t use the A.c below?

>>> +    +     ovsrcu_barrier();
>>> +    " \
>>> +    "WARNING: Are you sure you need to use ovsrcu_barrier(), "\
>>> +"in most cases ovsrcu_synchronize() will be fine?
>>> +    #8 FILE: a.c:1:
>>> +         ovsrcu_barrier();
>>> +"
>>> +
>>> +try_checkpatch \
>>> +   "PATCH_HEADER([lib/ovs-rcu.c])
>>> +    +     ovsrcu_barrier();
>>> +    "
>>> +
>>> +AT_CLEANUP
>>> +
>>>  AT_SETUP([checkpatch - whitespace around cast])
>>>  try_checkpatch \
>>>     "COMMON_PATCH_HEADER
>>> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
>>> index 8c7faa419..8c02ac3ce 100755
>>> --- a/utilities/checkpatch.py
>>> +++ b/utilities/checkpatch.py
>>> @@ -620,6 +620,10 @@ def regex_error_factory(description):
>>>      return lambda: print_error(description)
>>>
>>>
>>> +def regex_warn_factory(description):
>>> +    return lambda: print_warning(description)
>>> +
>>> +
>>>  std_functions = [
>>>          ('malloc', 'Use xmalloc() in place of malloc()'),
>>>          ('calloc', 'Use xcalloc() in place of calloc()'),
>>> @@ -636,6 +640,7 @@ std_functions = [
>>>          ('assert', 'Use ovs_assert() in place of assert()'),
>>>          ('error', 'Use ovs_error() in place of error()'),
>>>  ]
>>> +
>>>  checks += [
>>>      {'regex': r'(\.c|\.h)(\.in)?$',
>>>       'match_name': None,
>>> @@ -644,6 +649,21 @@ checks += [
>>>       'print': regex_error_factory(description)}
>>>      for (function_name, description) in std_functions]
>>>
>>> +easy_to_misuse_api = [
>>> +        ('ovsrcu_barrier',
>>> +            'lib/ovs-rcu.c',
>>> +            'Are you sure you need to use ovsrcu_barrier(), '
>>> +            'in most cases ovsrcu_synchronize() will be fine?'),
>>> +        ]
>>> +
>>> +checks += [
>>> +    {'regex': r'(\.c)(\.in)?$',
>>> +     'match_name': lambda x: x != location,
>>> +     'prereq': lambda x: not is_comment_line(x),
>>> +     'check': regex_function_factory(function_name),
>>> +     'print': regex_warn_factory(description)}
>>> +    for (function_name, location, description) in easy_to_misuse_api]
>>> +
>>>
>>>  def regex_operator_factory(operator):
>>>      regex = re.compile(r'^[^#][^"\']*[^ "]%s[^ "\'][^"]*' % operator)
>>> @@ -676,12 +696,22 @@ def get_file_type_checks(filename):
>>>      global checks
>>>      checkList = []
>>>      for check in checks:
>>> +        regex_check = True
>>> +        match_check = True
>>> +
>>>          if check['regex'] is None and check['match_name'] is None:
>>>              checkList.append(check)
>>> +            continue
>>> +
>>>          if check['regex'] is not None and \
>>> -           re.compile(check['regex']).search(filename) is not None:
>>> -            checkList.append(check)
>>> -        elif check['match_name'] is not None and
>> check['match_name'](filename):
>>> +           re.compile(check['regex']).search(filename) is None:
>>> +            regex_check = False
>>> +
>>> +        if check['match_name'] is not None and \
>>> +                not check['match_name'](filename):
>>> +            match_check = False
>>> +
>>> +        if regex_check and match_check:
>>>              checkList.append(check)
>>>      return checkList
>>>
>>> --
>>> 2.25.1
>>
>>
>
> -- 
> hepeng

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to