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
