From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A7565C43217 for ; Wed, 22 Sep 2021 02:25:57 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 47C9F61178 for ; Wed, 22 Sep 2021 02:25:57 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 47C9F61178 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvack.org Received: by kanga.kvack.org (Postfix) id 79E7D6B006C; Tue, 21 Sep 2021 22:25:56 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 74DD76B0072; Tue, 21 Sep 2021 22:25:56 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 6156C900002; Tue, 21 Sep 2021 22:25:56 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0120.hostedemail.com [216.40.44.120]) by kanga.kvack.org (Postfix) with ESMTP id 4FE106B006C for ; Tue, 21 Sep 2021 22:25:56 -0400 (EDT) Received: from smtpin10.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id 12E842C695 for ; Wed, 22 Sep 2021 02:25:56 +0000 (UTC) X-FDA: 78613619112.10.3BC38C1 Received: from mail-pf1-f173.google.com (mail-pf1-f173.google.com [209.85.210.173]) by imf24.hostedemail.com (Postfix) with ESMTP id B2808B0000A1 for ; Wed, 22 Sep 2021 02:25:55 +0000 (UTC) Received: by mail-pf1-f173.google.com with SMTP id q23so1358138pfs.9 for ; Tue, 21 Sep 2021 19:25:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=EYPmTFwbBM8fuIC7340+u45bIhCGBLoT/UxK4/aF7nk=; b=TFaCJt2y2zqQlkhVDjb7c8BaPXdlhPaEKCtoXHb2T2wQiroGttSgKzFtnrgLVruaDw J+Jag16UKxeqZduzZ4xEvTVQg07tq77BwNCwGSEtPmptzBod+N4ApxcUnBvc/HEfu0yJ X75omPNroUAgC4zFjzop9DXt3J21yEfJKAU2M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=EYPmTFwbBM8fuIC7340+u45bIhCGBLoT/UxK4/aF7nk=; b=huDsIMSQLbgXA5nFTvtUoCkf9wtbCfGtta69ydO7/0yIiwNtRGKSmHirnd5Khs8WM+ KFuoZ8v6VcXHKymeLPrhpFqzbzwKb3muA5TR8Fiy+diEssRePvHpu5WmwPBNQ1LP8vFT Yf7mGRZgV2kEQOjsGm18kRWFUzi2wBYecGre9tRR7rLlK2hJDc+K3jhnHQQJ1MxEZ3Zt s/XKfflhENkrqgPZ2kYAaeoKqdQdGA3HEj437aVBJfhOKDQNhny+bhCtGPvSyuVZmupK eNl0qCuamZ/iTn2gcBBl7lQ568w/UU9Ft0OhnAe4ZILbJEa1QT1SM3hn8SdN6juib59o oPaQ== X-Gm-Message-State: AOAM530KfZtGTzY2RC2JVo8Ok7nmgYJrcvi/8J+jdhJzFWSHQNPTbawY 01EwzQPay4sEMxvsNMMwFAfsqg== X-Google-Smtp-Source: ABdhPJxz0SutMentjbJobo7DWQY5POAP3HTNcaTOVTZ41K0G+D644hPUDDoWvL58U2CfuqMWpknYyA== X-Received: by 2002:a63:d0d:: with SMTP id c13mr30587088pgl.294.1632277554505; Tue, 21 Sep 2021 19:25:54 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id p2sm329269pja.51.2021.09.21.19.25.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Sep 2021 19:25:53 -0700 (PDT) Date: Tue, 21 Sep 2021 19:25:53 -0700 From: Kees Cook To: Linus Torvalds , Joe Perches Cc: linux-kernel@vger.kernel.org, Andrew Morton , apw@canonical.com, Christoph Lameter , Daniel Micay , Dennis Zhou , dwaipayanray1@gmail.com, Joonsoo Kim , Linux-MM , Lukas Bulwahn , mm-commits@vger.kernel.org, Nathan Chancellor , Nick Desaulniers , Miguel Ojeda , Pekka Enberg , David Rientjes , Tejun Heo , Vlastimil Babka , linux-doc@vger.kernel.org Subject: function prototype element ordering Message-ID: <202109211757.F38DF644@keescook> References: <20210909200948.090d4e213ca34b5ad1325a7e@linux-foundation.org> <20210910031046.G76dQvPhV%akpm@linux-foundation.org> <202109211630.2D00627@keescook> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: B2808B0000A1 X-Stat-Signature: dtjf1irbx43ayhmww77hzxcmu1a6zggp Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=chromium.org header.s=google header.b=TFaCJt2y; spf=pass (imf24.hostedemail.com: domain of keescook@chromium.org designates 209.85.210.173 as permitted sender) smtp.mailfrom=keescook@chromium.org; dmarc=pass (policy=none) header.from=chromium.org X-HE-Tag: 1632277555-980019 Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Sep 21, 2021 at 04:45:44PM -0700, Joe Perches wrote: > On Tue, 2021-09-21 at 16:37 -0700, Kees Cook wrote: > > On Fri, Sep 10, 2021 at 10:23:48AM -0700, Linus Torvalds wrote: > > > On Thu, Sep 9, 2021 at 8:10 PM Andrew Morton wrote: > > > >=20 > > > > +__alloc_size(1) > > > > =A0extern void *vmalloc(unsigned long size); > > > [...] > > >=20 > > > All of these are added in the wrong place - inconsistent with the v= ery > > > compiler documentation the patches add. > > >=20 > > > The function attributes are generally added _after_ the function, > > > although admittedly we've been quite confused here before. > > >=20 > > > But the very compiler documentation you point to in the patch that > > > adds these macros gives that as the examples both for gcc and clang= : > > >=20 > > > + * gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attri= butes.html#index-alloc_005fsize-function-attribute > > > + * clang: https://clang.llvm.org/docs/AttributeReference.html#allo= c-size > > >=20 > > > and honestly I think that is the preferred format because this is > > > about the *function*, not about the return type. > > >=20 > > > Do both placements work? Yes. > >=20 > > I'm cleaning this up now, and have discovered that the reason for the > > before-function placement is consistency with static inlines. If I do= this: > >=20 > > static __always_inline void * kmalloc(size_t size, gfp_t flags) __all= oc_size(1) > > { > > ... > > } > >=20 > > GCC is very angry: > >=20 > > ./include/linux/slab.h:519:1: error: attributes should be specified b= efore the declarator in a function definition > > =A0=A0519 | static __always_inline void *kmalloc_large(size_t size, g= fp_t flags) __alloc_size(1) > > =A0=A0=A0=A0=A0=A0| ^~~~~~ > >=20 > > It's happy if I treat it as a "return type attribute" in the ordering= , > > though: > >=20 > > static __always_inline void * __alloc_size(1) kmalloc(size_t size, gf= p_t flags) > >=20 > > I'll do that unless you have a preference for somewhere else... >=20 > _please_ put it before the return type on a separate line. >=20 > [__attributes] > [static inline const] function() Somehow Linus wasn't in CC. :P Linus, what do you want here? I keep getting conflicting (or uncompilable) advice. I'm also trying to prepare a patch for Documentation/process/coding-style.rst ... 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. An= d 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? 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): [storage class] [storage class attributes] [return type] [return type att= ributes] [function argument attributes] [name]([arg1type] [arg1name], ...= ) [function attributes] Joe appears to want (on two lines): [storage class attributes] [function attributes] [function argument attri= butes] [storage class] [return type] [return type attributes] [name]([arg1type] = [arg1name], ...) I would just like to have an arrangement that won't get NAKed by someone. ;) And I'm willing to document it. :) -Kees [1] https://lore.kernel.org/mm-commits/CAHk-=3DwiOCLRny5aifWNhr621kYrJwhf= URsa0vFPeUEm8mF0ufg@mail.gmail.com/ --=20 Kees Cook