On 23 Jun 2026, at 23:42, Aaron Conole wrote:
> 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.
Ack, yes we can keep it as is, not worth the rework for this corner case.
>>> +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
Thanks.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
