have to work the unit test for ovsrcu_barrier API. will send all the fix in the next version.
Eelco Chaudron <[email protected]> 于2022年5月25日周三 20:44写道： > > > On 24 May 2022, at 16:29, Peng He wrote: > > > Signed-off-by: Peng He <[email protected]> > > Signed-off-by: Peng He <[email protected]> > > This patch is making existing test fail: > > 10: checkpatch - sign-offs ok > 11: checkpatch - parenthesized constructs FAILED ( > checkpatch.at:27) > 12: checkpatch - catastrophic backtracking FAILED ( > checkpatch.at:27) > 13: checkpatch - parenthesized constructs - for FAILED ( > checkpatch.at:27) > 14: checkpatch - comments FAILED ( > checkpatch.at:27) > 15: checkpatch - whitespace around operator FAILED ( > checkpatch.at:27) > 16: checkpatch - check misuse APIs ok > 17: checkpatch - whitespace around cast FAILED ( > checkpatch.at:27) > 18: checkpatch - malformed tags ok > 19: checkpatch - Unicode code FAILED ( > checkpatch.at:27) > > See also the fix below, which was what I had in mind asking for a single > macro. > > > --- > > tests/checkpatch.at | 30 ++++++++++++++++++++++++++---- > > utilities/checkpatch.py | 36 +++++++++++++++++++++++++++++++++--- > > 2 files changed, 59 insertions(+), 7 deletions(-) > > > > diff --git a/tests/checkpatch.at b/tests/checkpatch.at > > index fadb625e9..4dceb8b26 100755 > > --- a/tests/checkpatch.at > > +++ b/tests/checkpatch.at > > @@ -159,17 +159,21 @@ try_checkpatch \ > > AT_CLEANUP > > > > > > -m4_define([COMMON_PATCH_HEADER], [dnl > > +m4_define([_COMMON_PATCH_HEADER], [dnl > > Author: A > > > > Signed-off-by: A > > --- > > - diff --git a/A.c b/A.c > > + diff --git a/$1 b/$1 > > index 0000000..1111111 100644 > > - --- a/A.c > > - +++ b/A.c > > + --- a/$1 > > + +++ b/$1 > > @@ -1,1 +1,1 @@]) > > > > +m4_define([COMMON_PATCH_HEADER], > > + [_COMMON_PATCH_HEADER([A.c]) > > +]) > > + > > > I was hoping for a more simple solution, i.e. if $1 is not present use A.c > or else the value given. > This is a diff I used to also make all existing tests pass: > > diff --git a/tests/checkpatch.at b/tests/checkpatch.at > index 4dceb8b26..5b326b083 100755 > --- a/tests/checkpatch.at > +++ b/tests/checkpatch.at > @@ -159,24 +159,20 @@ try_checkpatch \ > AT_CLEANUP > > > -m4_define([_COMMON_PATCH_HEADER], [dnl > +m4_define([COMMON_PATCH_HEADER], [dnl > Author: A > > Signed-off-by: A > --- > - diff --git a/$1 b/$1 > + diff --git a/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c") b/$([[ ! > -z "$1" ]] && echo "$1" || echo "A.c") > index 0000000..1111111 100644 > - --- a/$1 > - +++ b/$1 > + --- a/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c") > + +++ b/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c") > @@ -1,1 +1,1 @@]) > > -m4_define([COMMON_PATCH_HEADER], > - [_COMMON_PATCH_HEADER([A.c]) > -]) > - > - > AT_SETUP([checkpatch - parenthesized constructs]) > for ctr in 'if' 'while' 'switch' 'HMAP_FOR_EACH' 'BITMAP_FOR_EACH_1'; do > + > try_checkpatch \ > "COMMON_PATCH_HEADER > + $ctr (first_run) { > @@ -354,7 +350,7 @@ AT_CLEANUP > > AT_SETUP([checkpatch - check misuse APIs]) > try_checkpatch \ > - "_COMMON_PATCH_HEADER([a.c]) > + "COMMON_PATCH_HEADER([a.c]) > + ovsrcu_barrier(); > " \ > "WARNING: Are you sure you need to use ovsrcu_barrier(), "\ > @@ -364,7 +360,7 @@ try_checkpatch \ > " > > try_checkpatch \ > - "_COMMON_PATCH_HEADER([lib/ovs-rcu.c]) > + "COMMON_PATCH_HEADER([lib/ovs-rcu.c]) > + ovsrcu_barrier(); > " > > I was trying string substitution but that did not work with the framework, > i.e. $(1:-A.c}. > Not sure if this $() is the best way, maybe there is a more elegant way. > > > > > AT_SETUP([checkpatch - parenthesized constructs]) > > for ctr in 'if' 'while' 'switch' 'HMAP_FOR_EACH' 'BITMAP_FOR_EACH_1'; do > > @@ -348,6 +352,24 @@ try_checkpatch \ > > > > AT_CLEANUP > > > > +AT_SETUP([checkpatch - check misuse APIs]) > > +try_checkpatch \ > > + "_COMMON_PATCH_HEADER([a.c]) > > See above patch, make this just COMMON_PATCH_HEADER, with not parameters. > > > + + 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: > > See above, make it A.c > > > + ovsrcu_barrier(); > > +" > > + > > +try_checkpatch \ > > + "_COMMON_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
