Eelco Chaudron <[email protected]> writes:
> On 23 Jun 2026, at 21:21, Aaron Conole wrote:
>
>> 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).
>>
>> Signed-off-by: Aaron Conole <[email protected]>
>>
>> 0:
>> https://bylizhao.github.io/c/programming/2020/12/12/Static-inline-considered-harmful.html
>> ---
>
> Thanks for sorting this out Aaron!
Thanks for the quick turnaround!
> See some comments below.
>
> //Eelco
>
>> tests/checkpatch.at | 70 +++++++++++++++++++++++++++++++++++++++++
>> utilities/checkpatch.py | 11 ++++---
>> 2 files changed, 77 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/checkpatch.at b/tests/checkpatch.at
>> index 7d80ffbed5..a1b915fa48 100755
>> --- a/tests/checkpatch.at
>> +++ b/tests/checkpatch.at
>> @@ -750,6 +750,76 @@ 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() {
>> + }" \
>> + "ERROR: Using 'static inline' in a c-file.
>> + test.c:4:
>> + static inline bool foo() {"
>> +
>> +try_checkpatch_c_file \
>> + "#include <foo.h>
>> + #include <bar.h>
>> +
>> + static inline
>> + bool foo() {
>> + }" \
>> + "ERROR: Using 'static inline' in a c-file.
>> + test.c:4:
>> + static inline"
>> +
>> +try_checkpatch_c_file \
>> + "#include <foo.h>
>> + #include <bar.h>
>> +
>> + static inline
>> + bool foo() {
>> + }" \
>> + "ERROR: Using 'static inline' in a c-file.
>> + test.c:4:
>> + static inline"
>> +
>> +try_checkpatch_c_file \
>> + "#include <foo.h>
>> + #include <bar.h>
>> +
>> + static inline
>> + bool foo() {
>> + }" \
>> + "ERROR: Using 'static inline' in a c-file.
>> + test.c:4:
>> + static 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() {
>> + }"
>> +
>
> nit; Lots of tests ;) but if static and inline are split it fails:
>
> +try_checkpatch_c_file \
> + "#include <foo.h>
> + #include <bar.h>
> +
> + static
> + inline
> + bool foo() {
> + }" \
> + "ERROR: Using 'static inline' in a c-file.
> + test.c:4:
> + static inline"
Yes - unfortunately it is kindof how the check system works for source
code. Checks are executed 'line-at-a-time' for parsing states. We
would need to write something that would keep additional line state
across lines, or change how the check system worked to support this
case. I thought about the case a little as well, and if someone submits
it to try and defeat the checkpatch alert, our inferior human eyeballs
and puny coding spidey-sense will hopefully catch it. It looks jarring.
>> +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 113c371098..66a9a86769 100755
>> --- a/utilities/checkpatch.py
>> +++ b/utilities/checkpatch.py
>> @@ -162,6 +162,7 @@ __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')
>>
>> skip_leading_whitespace_check = False
>> skip_trailing_whitespace_check = False
>> @@ -629,6 +630,12 @@ 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,
>> + 'print':
>> + lambda: print_error("Using 'static inline' in a c-file.")
>> + },
>> +
>> {'regex': r'(\.at|\.sh)$', 'match_name': None,
>> 'check': lambda x: has_efgrep(x),
>> 'print':
>> @@ -637,10 +644,6 @@ checks = [
>> {'regex': 'AUTHORS.rst$', 'match_name': None,
>> 'check': lambda x: update_missing_authors(x),
>> 'print': None},
>> -
>> - {'regex': None, 'match_name': None,
>> - 'check': lambda x: 'ALLOW_EXPERIMENTAL_API' in x,
>> - 'print': lambda: print_error("DPDK Experimental API is not allowed")},
>
> Guess this should not have been deleted?
d'oh! I'll fix in v2
>> ]
>>
>>
>> --
>> 2.51.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
