there are two tests:

1) check the usage of this api in a normal file
2) if the api is defined in its own file, which is specified in the
'location' field in checkpatch.py, the warning should be skipped.

So, either use a new macro, either make COMMON_PATCH_HEADER a macro with a
specified filename.

Eelco Chaudron <[email protected]> 于2022年5月23日周一 17:07写道:

>
>
> 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
>
>

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

Reply via email to