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?
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
