On 9 Jul 2026, at 13:46, Dmitry Mityugov wrote:
> On Wed, Jun 24, 2026 at 5:04 PM Eelco Chaudron via dev > <[email protected]> wrote: >> >> >> >> 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. > > By the way, static inline functions are still marked as Ok in the > Style Guide. Shouldn't it be fixed together with this patch? If this is the case, we should probably update the documentation. Aaron WDYT? > Sorry if I'm off the track > >>>>> +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 > > > > -- > Dmitry _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
