Modern optimizing compilers will automatically choose to inline when it is appropriate for specific functions. In fact, 'inline' is usually considered harmful[0] except in specific situations (such as using them in header files to prevent linker errors when the One Definition Rule needs to be relaxed).
Additionally, the coding style guidelines strongly recommend against using static inline in a C file, and we should strengthen that by also requiring such instances be decorated with ALWAYS_INLINE. Issue a warning instead of an error because the checkpatch parsing system doesn't support multi-line code blocks properly, so it's best to keep this a warning to call attention to the instance of ``static inline`` without explicitly calling it an error. Signed-off-by: Aaron Conole <[email protected]> --- v1->v2: * Update the documentation for ``static inline`` in ``.c`` files. * Drop the unrelated 'check' change * Change from ERROR to WARNING (since there are multi-line cases) * Accept static inlines that have ALWAYS_INLINE decorated. .../internals/contributing/coding-style.rst | 4 +- tests/checkpatch.at | 100 ++++++++++++++++++ utilities/checkpatch.py | 11 ++ 3 files changed, 114 insertions(+), 1 deletion(-) diff --git a/Documentation/internals/contributing/coding-style.rst b/Documentation/internals/contributing/coding-style.rst index 034fd4cef0..af022b6f7c 100644 --- a/Documentation/internals/contributing/coding-style.rst +++ b/Documentation/internals/contributing/coding-style.rst @@ -209,7 +209,9 @@ null-pointer check. We find that this usually makes code easier to read. Functions in ``.c`` files should not normally be marked ``inline``, because it does not usually help code generation and it does suppress compiler warnings about unused functions. (Functions defined in ``.h`` usually should be marked -``inline``.) +``inline``.) In cases where a ``.c`` file genuinely requires using +``static inline`` functions, they should also be decorated with +``ALWAYS_INLINE``. .. _function prototypes: diff --git a/tests/checkpatch.at b/tests/checkpatch.at index 7d80ffbed5..aa5d67f832 100755 --- a/tests/checkpatch.at +++ b/tests/checkpatch.at @@ -750,6 +750,106 @@ try_checkpatch_c_file \ done AT_CLEANUP +AT_SETUP([checkpatch - file contents checks - static inline in C files]) + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + static inline bool foo() { + }" \ + "WARNING: Using 'static inline' in a c-file without ALWAYS_INLINE. + test.c:4: + static inline bool foo() {" + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + static inline + bool foo() { + }" \ + "WARNING: Using 'static inline' in a c-file without ALWAYS_INLINE. + test.c:4: + static inline" + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + static inline + bool foo() { + }" \ + "WARNING: Using 'static inline' in a c-file without ALWAYS_INLINE. + test.c:4: + static inline" + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + static inline + bool foo() { + }" \ + "WARNING: Using 'static inline' in a c-file without ALWAYS_INLINE. + test.c:4: + static inline" + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + static inline ALWAYS_INLINE + bool foo() { + }" + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + ALWAYS_INLINE static inline + bool foo() { + }" + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + ALWAYS_INLINE static inline bool foo() { + }" + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + static inline bool foo() ALWAYS_INLINE { + }" + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + static void inliner() { + }" + +try_checkpatch_c_file \ + "#include <foo.h> + #include <bar.h> + + static void inliner() { + }" + +dnl This is to confirm that filename selection works +try_checkpatch_py_file \ + "#include <foo.h> + #include <bar.h> + + static inline bool foo() { + }" + +AT_CLEANUP + + AT_SETUP([checkpatch - spelling checks]) AT_SKIP_IF([! $PYTHON3 -c 'import enchant' >/dev/null 2>&1]) diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index 3edd8192a4..387a7bea10 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -162,6 +162,8 @@ __regex_if_macros = re.compile(r'^ +(%s) \([\S](?:[\s\S]*?[\S])?\) { +\\' % __parenthesized_constructs) __regex_nonascii_characters = re.compile("[^\u0000-\u007f]") __regex_efgrep = re.compile(r'.*[ef]grep.*$') +__regex_static_inline_c_file = re.compile(r'^\s*static\s+inline') +__regex_always_inline = re.compile(r'\s*ALWAYS_INLINE\s*') skip_leading_whitespace_check = False skip_trailing_whitespace_check = False @@ -629,6 +631,15 @@ checks = [ lambda: print_warning("Empty return followed by brace, consider omitting") }, + {'regex': r'(\.c)(\.in)?$', 'match_name': None, + 'check': + lambda x: __regex_static_inline_c_file.search(x) is not None and + __regex_always_inline.search(x) is None, + 'print': + lambda: print_warning( + "Using 'static inline' in a c-file without ALWAYS_INLINE.") + }, + {'regex': r'(\.at|\.sh)$', 'match_name': None, 'check': lambda x: has_efgrep(x), 'print': -- 2.51.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
