Message ID | 20220517014756.437822-3-hepeng.0320@bytedance.com |
---|---|
State | Superseded |
Headers | show |
Series | [ovs-dev,ovs-dev,v4,1/3] ovs-rcu: add rcu_barrier | expand |
Context | Check | Description |
---|---|---|
ovsrobot/apply-robot | warning | apply and check: warning |
ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
ovsrobot/intel-ovs-compilation | success | test: success |
Bleep bloop. Greetings Peng He, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: ERROR: Author Peng He <xnhp0320@gmail.com> needs to sign off. WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Peng He <hepeng.0320@bytedance.com> Lines checked: 86, Warnings: 1, Errors: 1 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On 17 May 2022, at 3:47, Peng He wrote: > Signed-off-by: Peng He <hepeng.0320@bytedance.com> > Acked-by: Eelco Chaudron <echaudro@redhat.com> > --- Changes look good to me. Acked-by: Eelco Chaudron <echaudro@redhat.com>
Peng He <xnhp0320@gmail.com> writes: > Signed-off-by: Peng He <hepeng.0320@bytedance.com> > Acked-by: Eelco Chaudron <echaudro@redhat.com> > --- Sorry it took so long to review. I like this feature, and thanks for the concise implementation. Please add tests into tests/checkpatch.at to ensure that a future change doesn't break it :) With such a test, you can also add my Acked-by: Aaron Conole <aconole@redhat.com> > utilities/checkpatch.py | 36 +++++++++++++++++++++++++++++++++--- > 1 file changed, 33 insertions(+), 3 deletions(-) > > 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
Thanks, Aaron. sorry for the late reply. I now have added the tests for checkpatch.py. will send a new version. Aaron Conole <aconole@redhat.com> 于2022年5月17日周二 21:02写道: > Peng He <xnhp0320@gmail.com> writes: > > > Signed-off-by: Peng He <hepeng.0320@bytedance.com> > > Acked-by: Eelco Chaudron <echaudro@redhat.com> > > --- > > Sorry it took so long to review. I like this feature, and thanks for > the concise implementation. > > Please add tests into tests/checkpatch.at to ensure that a future > change doesn't break it :) > > With such a test, you can also add my > > Acked-by: Aaron Conole <aconole@redhat.com> > > > utilities/checkpatch.py | 36 +++++++++++++++++++++++++++++++++--- > > 1 file changed, 33 insertions(+), 3 deletions(-) > > > > 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 > >
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