On 7/15/26 3:57 PM, 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).
>
> 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() {
> + }"
nit: These are two identical checks.
> +
> +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
> +
> +
nit: Should be one line. The other tests have one in between.
> AT_SETUP([checkpatch - spelling checks])
> AT_SKIP_IF([! $PYTHON3 -c 'import enchant' >/dev/null 2>&1])
>
Thanks, Aaron and Eelco! I removed the duplicate check and the extra
empty line and applied the change.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
