diff mbox series

[ovs-dev,ovs-dev,v7,3/3] checkpatch.py: Add checks for easy-to-misuse APIs.

Message ID 20220526031318.559449-2-hepeng.0320@bytedance.com
State Accepted
Commit 071b802c61a3bf0b9304764b4cbc10dfad972105
Headers show
Series None | expand

Commit Message

Peng He May 26, 2022, 3:13 a.m. UTC 
Signed-off-by: Peng He <hepeng.0320@bytedance.com>
Signed-off-by: Peng He <xnhp0320@gmail.com>
---
 tests/checkpatch.at     | 24 +++++++++++++++++++++---
 utilities/checkpatch.py | 36 +++++++++++++++++++++++++++++++++---
 2 files changed, 54 insertions(+), 6 deletions(-)

Comments

Eelco Chaudron May 27, 2022, 8:14 a.m. UTC | #1 
On 26 May 2022, at 5:13, Peng He wrote:

> Signed-off-by: Peng He <hepeng.0320@bytedance.com>
> Signed-off-by: Peng He <xnhp0320@gmail.com>
> ---
>  tests/checkpatch.at     | 24 +++++++++++++++++++++---
>  utilities/checkpatch.py | 36 +++++++++++++++++++++++++++++++++---
>  2 files changed, 54 insertions(+), 6 deletions(-)

Changes look good, and tests are passing now!

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Aaron Conole May 27, 2022, 3:03 p.m. UTC | #2 
Peng He <xnhp0320@gmail.com> writes:

> Signed-off-by: Peng He <hepeng.0320@bytedance.com>
> Signed-off-by: Peng He <xnhp0320@gmail.com>
> ---

Acked-by: Aaron Conole <aconole@redhat.com>
diff mbox series

Patch

diff --git a/tests/checkpatch.at b/tests/checkpatch.at
index fadb625e9..0a2cca34e 100755
--- a/tests/checkpatch.at
+++ b/tests/checkpatch.at
@@ -164,10 +164,10 @@  m4_define([COMMON_PATCH_HEADER], [dnl
 
     Signed-off-by: A
     ---
-    diff --git a/A.c b/A.c
+    diff --git a/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c") b/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c")
     index 0000000..1111111 100644
-    --- a/A.c
-    +++ b/A.c
+    --- a/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c")
+    +++ b/$([[ ! -z "$1" ]] && echo "$1" || echo "A.c")
     @@ -1,1 +1,1 @@])
 
 
@@ -348,6 +348,24 @@  try_checkpatch \
 
 AT_CLEANUP
 
+AT_SETUP([checkpatch - check misuse APIs])
+try_checkpatch \
+   "COMMON_PATCH_HEADER([a.c])
+    +     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 \
+   "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