| Message ID | 20210924202302.2335542-1-keescook@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | docs: Explain the desired position of function attributes | expand |
On 9/24/21 1:23 PM, Kees Cook wrote: > While discussing how to format the addition of various function > attributes, some "unwritten rules" of ordering surfaced[1]. Capture a > synthesized version of Linus's, Joe's, and Rasmus's recommendations on > this subject for future reference. > > [1] https://lore.kernel.org/mm-commits/CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com > > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > Documentation/process/coding-style.rst | 27 ++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst > index 42969ab37b34..3559c34a9281 100644 > --- a/Documentation/process/coding-style.rst > +++ b/Documentation/process/coding-style.rst > @@ -487,6 +487,33 @@ 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 prototype, please keep the `order of elements regular > +<https://lore.kernel.org/mm-commits/CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com>`_. For example:: > + > + __must_check __printf(4, 5) __malloc __init > + static __always_inline void *action(enum magic value, size_t size, > + u8 count, char *fmt, ...) > + { > + ... > + } > + > +The preferred order of elements for a function prototype is: > + > +- attributes on the preceding lines > + I thought that idea was already nacked: (it's more of a BSD thing AFAIK) (and I would NAK it if I could :) """ > Attributes should be on their own line, they can be quite lengthy. No, no no. They really shouldn't. "" from: https://lore.kernel.org/mm-commits/CAHk-=wjS-Jg7sGMwUPpDsjv392nDOOs0CtUtVkp=S6Q7JzFJRw@mail.gmail.com/ > + - return type attributes (here, ``__must_check``) > + - function parameter attributes (here, ``__printf(4,5)``) > + - function behavior attributes (here, ``__malloc``) > + - storage class attributes (here, ``__init``) > + > +- main function prototype on the next lines > + > + - storage class (here, ``static __always_inline`` -- even though > + ``__always_inline`` is technically an attribute, it is treated like > + ``inline``) > + - return type (here, ``void *``) > + - function name (here, ``action``) > + - function parameters (as described earlier: each with type and name) > > 7) Centralized exiting of functions > ----------------------------------- >
On Fri, Sep 24, 2021 at 01:27:20PM -0700, Randy Dunlap wrote: > On 9/24/21 1:23 PM, Kees Cook wrote: > > +The preferred order of elements for a function prototype is: > > + > > +- attributes on the preceding lines > > + > > I thought that idea was already nacked: (it's more of a BSD thing AFAIK) > (and I would NAK it if I could :) > > """ > > Attributes should be on their own line, they can be quite lengthy. > > No, no no. They really shouldn't. > "" > > from: https://lore.kernel.org/mm-commits/CAHk-=wjS-Jg7sGMwUPpDsjv392nDOOs0CtUtVkp=S6Q7JzFJRw@mail.gmail.com/ Right -- and then Joe and Rasmus had some convincing counter-arguments, IMO. So, given the outlined Docs patch, I'd like to see what folks can propose in the form of alternative patches for this topic. I genuinely don't care. I just want to have something I can follow for the refactoring of the allocator attributes. :) The trouble I had with Linus's suggestion is that some attributes don't work[1] at the end for function definitions, so I'm left unable to follow his recommendations too. -Kees [1] https://lore.kernel.org/mm-commits/202109211630.2D00627@keescook/
diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst index 42969ab37b34..3559c34a9281 100644 --- a/Documentation/process/coding-style.rst +++ b/Documentation/process/coding-style.rst @@ -487,6 +487,33 @@ 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 prototype, please keep the `order of elements regular +<https://lore.kernel.org/mm-commits/CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com>`_. For example:: + + __must_check __printf(4, 5) __malloc __init + static __always_inline void *action(enum magic value, size_t size, + u8 count, char *fmt, ...) + { + ... + } + +The preferred order of elements for a function prototype is: + +- attributes on the preceding lines + + - return type attributes (here, ``__must_check``) + - function parameter attributes (here, ``__printf(4,5)``) + - function behavior attributes (here, ``__malloc``) + - storage class attributes (here, ``__init``) + +- main function prototype on the next lines + + - storage class (here, ``static __always_inline`` -- even though + ``__always_inline`` is technically an attribute, it is treated like + ``inline``) + - return type (here, ``void *``) + - function name (here, ``action``) + - function parameters (as described earlier: each with type and name) 7) Centralized exiting of functions -----------------------------------
While discussing how to format the addition of various function attributes, some "unwritten rules" of ordering surfaced[1]. Capture a synthesized version of Linus's, Joe's, and Rasmus's recommendations on this subject for future reference. [1] https://lore.kernel.org/mm-commits/CAHk-=wiOCLRny5aifWNhr621kYrJwhfURsa0vFPeUEm8mF0ufg@mail.gmail.com Signed-off-by: Kees Cook <keescook@chromium.org> --- Documentation/process/coding-style.rst | 27 ++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)