Re: [PATCH v4] docs: Explain the desired position of function attributes
From: Nick Desaulniers <ndesaulniers@google.com>
Date: 2021-10-01 19:05:42
Also in:
linux-hardening, lkml
On Thu, Sep 30, 2021 at 4:58 PM Kees Cook [off-list ref] wrote:
While discussing how to format the addition of various function attributes, some "unwritten rules" of ordering surfaced[1]. Capture as close as possible to Linus's preferences for future reference. (Though I note the dissent voiced by Joe Perches, Alexey Dobriyan, and others that would prefer all attributes live on a separate leading line.) [1] https://lore.kernel.org/mm-commits/CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com/ (local) Signed-off-by: Kees Cook <redacted>
While I appreciate you getting the ball across the finish line (having _any_ documentation to point to in future bikesheds), I can't help but shake the feeling that the chosen policy will harm the ability of existing automated code formatting tools from being able to automate code formatting on the kernel.
quoted hunk ↗ jump to hunk
--- v4: - fix another stray "void"! This is why code needs a compiler... (thx randy) --- Documentation/process/coding-style.rst | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst index 42969ab37b34..45b48510f5ec 100644 --- a/Documentation/process/coding-style.rst +++ b/Documentation/process/coding-style.rst@@ -487,6 +487,36 @@ because it is a simple way to add valuable information for the reader. Do not use the ``extern`` keyword with function prototypes as this makes lines longer and isn't strictly necessary. +When writing a function declarations, please keep the `order of elements regular +<https://lore.kernel.org/mm-commits/CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com/>`_. +For example:: + + extern __init void * __must_check action(enum magic value, size_t size, u8 count, + char *fmt, ...) __printf(4, 5) __malloc; + +The preferred order of elements for a function prototype is: + +- storage class (here, ``extern``, and things like ``static __always_inline`` even though + ``__always_inline`` is technically an attribute, it is treated like ``inline``) +- storage class attributes (here, ``__init`` -- i.e. section declarations, but also things like ``__cold``) +- return type (here, ``void *``) +- return type attributes (here, ``__must_check``) +- function name (here, ``action``) +- function parameters (here, ``(enum magic value, size_t size, u8 count, char *fmt, ...)``, noting that parameter names should always be included) +- function parameter attributes (here, ``__printf(4, 5)``) +- function behavior attributes (here, ``__malloc``) + +Note that for a function definition (e.g. ``static inline``), the compiler does +not allow function parameter attributes after the function parameters. In these +cases, they should go after the storage class attributes (e.g. note the changed +position of ``__printf(4, 5)``):: + + static __always_inline __init __printf(4, 5) void * __must_check action(enum magic value, + size_t size, u8 count, char *fmt, ...) + __malloc + { + ... + } 7) Centralized exiting of functions ----------------------------------- --2.30.2
-- Thanks, ~Nick Desaulniers