Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 24 Sep 2021 12:43:38 -0700
From:   Kees Cook <keescook@chromium.org>
To:     Joe Perches <joe@perches.com>
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        linux-kernel@vger.kernel.org,
        Andrew Morton <akpm@linux-foundation.org>, apw@canonical.com,
        Christoph Lameter <cl@linux.com>,
        Daniel Micay <danielmicay@gmail.com>,
        Dennis Zhou <dennis@kernel.org>, dwaipayanray1@gmail.com,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Linux-MM <linux-mm@kvack.org>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>,
        mm-commits@vger.kernel.org, Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>,
        Miguel Ojeda <ojeda@kernel.org>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Tejun Heo <tj@kernel.org>, Vlastimil Babka <vbabka@suse.cz>,
        linux-doc@vger.kernel.org
Subject: Re: function prototype element ordering
Message-ID: <202109241238.13C92BA004@keescook>
References: <20210909200948.090d4e213ca34b5ad1325a7e@linux-foundation.org>
 <20210910031046.G76dQvPhV%akpm@linux-foundation.org>
 <CAHk-=wgfbSyW6QYd5rmhSHRoOQ=ZvV+jLn1U8U4nBDgBuaOAjQ@mail.gmail.com>
 <202109211630.2D00627@keescook>
 <af3c775a1515f97c8dbe6a6651bd6e4b6986e8cd.camel@perches.com>
 <202109211757.F38DF644@keescook>
 <0b1c78b395a7a198a089ba8f6283d8d10829720c.camel@perches.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <0b1c78b395a7a198a089ba8f6283d8d10829720c.camel@perches.com>
Precedence: bulk

On Tue, Sep 21, 2021 at 09:24:04PM -0700, Joe Perches wrote:
> On Tue, 2021-09-21 at 19:25 -0700, Kees Cook wrote:
> > [...]
> > Looking through what was written before[1] and through examples in the
> > source tree, I find the following categories:
> > 
> > 1- storage class: static extern inline __always_inline
> > 2- storage class attributes/hints/???: __init __cold
> > 3- return type: void *
> > 4- return type attributes: __must_check __noreturn __assume_aligned(n)
> > 5- function attributes: __attribute_const__ __malloc
> > 6- function argument attributes: __printf(n, m) __alloc_size(n)
> > 
> > Everyone seems to basically agree on:
> > 
> > [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)
> > 
> > There is a lot of disagreement over where 5 and 6 should fit in above. And
> > there is a lot of confusion over 4 (mixed between before and after the
> > function name) and 2 (see below).
> > 
> > What's currently blocking me is that 6 cannot go after the function
> > (for definitions) because it angers GCC (see quoted bit above), but 5
> > can (e.g. __attribute_const__).
> > 
> > Another inconsistency seems to be 2 (mainly section markings like
> > __init). Sometimes it's after the storage class and sometimes after the
> > return type, but it certainly feels more like a storage class than a
> > return type attribute:
> > 
> > $ git grep 'static __init int' | wc -l
> > 349
> > $ git grep 'static int __init' | wc -l
> > 8402
> > 
> > But it's clearly positioned like a return type attribute in most of the
> > tree. What's correct?
> 
> Neither really.  'Correct' is such a difficult concept.
> 'Preferred' might be better.

Right -- I expect it to be a guideline.

> btw: there are about another 100 other uses with '__init' as the
> initial attribute, mostly in trace.

Hah, yeah.

> And I still think that return type attributes like __init, which is
> just a __section define, should go before the function storage class
> and ideally on a separate line to simplify the parsing of the actual
> function declaration.  Attributes like __section, __aligned, __cold,
> etc... don't have much value when looking up a function definition.
> 
> > Regardless, given the constraints above, it seems like what Linus may
> > want is (on "one line", though it will get wrapped in pathological cases
> > like kmem_cache_alloc_node_trace):
> 
> Pathological is pretty common these days as the function name length
> is rather longer now than earlier times.

Agreed!

> > [storage class] [storage class attributes] [return type] [return type attributes] [function argument attributes] [name]([arg1type] [arg1name], ...) [function attributes]
> > 
> > Joe appears to want (on two lines):
> > 
> > [storage class attributes] [function attributes] [function argument attributes]
> > [storage class] [return type] [return type attributes] [name]([arg1type] [arg1name], ...)
> 
> I would put [return type attributes] on the initial separate line
> even though that's not the most common use today.

I found a few other people wanting separate lines too, so at the risk of
annoying Linus, I guess I'll attempt this (again).

> > I would just like to have an arrangement that won't get NAKed by
> > someone. ;)
> 
> Bikeshed building dreamer...

I just want to know the right place to put stuff. :P

> But IMO the desire here is to ask for a bit more uniformity, not
> require it.

Yeah.

-- 
Kees Cook