diff mbox series

[ovs-dev,ovs-dev,v4,3/3] checkpatch.py: add checks for experimental API.

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

Checks

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

Commit Message

Peng He May 17, 2022, 1:47 a.m. UTC
Signed-off-by: Peng He <hepeng.0320@bytedance.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>
---
 utilities/checkpatch.py | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

Comments

0-day Robot May 17, 2022, 2:02 a.m. UTC | #1
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
Eelco Chaudron May 17, 2022, 6:39 a.m. UTC | #2
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>
Aaron Conole May 17, 2022, 1:02 p.m. UTC | #3
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
Peng He May 20, 2022, 8:29 a.m. UTC | #4
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 mbox series

Patch

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