mm-commits.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: David Laight <David.Laight@aculab.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Joe Perches <joe@perches.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	"apw@canonical.com" <apw@canonical.com>,
	Christoph Lameter <cl@linux.com>,
	Daniel Micay <danielmicay@gmail.com>,
	Dennis Zhou <dennis@kernel.org>,
	"dwaipayanray1@gmail.com" <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" <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>,
	"open list:DOCUMENTATION" <linux-doc@vger.kernel.org>
Subject: Re: function prototype element ordering
Date: Sun, 26 Sep 2021 14:03:56 -0700	[thread overview]
Message-ID: <CAHk-=wjoLAYG446ZNHfg=GhjSY6nFmuB_wA8fYd5iLBNXjo9Bw@mail.gmail.com> (raw)
In-Reply-To: <6a85bbbf952949118cc5f93b57d48265@AcuMS.aculab.com>

On Sat, Sep 25, 2021 at 12:40 PM David Laight <David.Laight@aculab.com> wrote:
>
> If the function name starts at the beginning of a line it is
> much easier to grep for the definition.

That has always been a completely bogus argument. I grep to look up
the type as often as I grep for the function definition, plus it's not
at all unlikely that the "function" is actually a macro wrapper, so
grepping for the beginning of line is just completely wrong.

It's completely wrong for another reason too: it assumes a style of
programming that has never actually been all that common. It's a very
specific pattern to very specific projects, and anybody who learnt
that pattern for their project is going to be completely lost anywhere
else. So don't do it. It's just a bad idea.

So a broken "easier to grep for" is not an excuse for "make the code
harder to read" particularly when it just makes another type of
grepping harder, and it's not actually nearly universal enough to
actually be a useful pattern in the first place.

It's not only never been the pattern in the kernel, but it's generally
not been the pattern anywhere else either. It's literally one of the
broken GNU coding standards - and the fact that almost every other
part of the GNU coding standards were wrong (indentation, placement of
braces, you name it) should give you a hint about how good _that_ one
was.

Here's an exercise for you: go search for C coding examples on the
web, and see how many of them do

    int main(int argc, char **argv)

vs how many of them do

    int
    main(int argc, char **argv)

and then realize that in order for the "grep for ^main" pattern to be
useful, the second version has to not just be more common, it has to
be practically *universal*.

Hint: it isn't even remotely more common, much less universal. In
Debian code search, I had to go to the third page to find any example
at all of people putting the "int" and the "main" on different lines,
and even that one didn't place the "main()" at the beginning of the
line - they had been separated because of other reasons and looked
like this:

int
#ifdef _WIN32
    __cdecl
#endif // _WIN32
    main(int argc, char** argv)

instead.

Maybe Dbian code search isn't the place to go, but I think it proves
my case: the "function name at beginning of line" story is pure
make-believe, and has absolutely no relevance in the real world.

It's a bad straightjacket. Just get over it, and stop perpetuating the
idiotic myth.

If you care so much about grepping for function declarations, and you
use that old-fashioned GNU coding standard policy as an argument, just
be *properly* old-fashioned instead, and use etags or something.

Don't make the rest of us suffer.

Because I grep for functions all the time, and I'd rather have useful
output - which very much includes the type of the function. That's
often one reason _why_ I grep for things in the first place.

Other grep tricks for when the function really is used everywhere, and
you are having trouble finding the definition vs the use:

 - grep in the headers for the type, and actually use the type (either
of the function, or the first argument) as part of the pattern.

   If you really have no idea where it might be, you'll want to start
off with the header grep anyway, to find the macro case (or the inline
case)

   Yeah, splitting the declaration will screw the type information up.
So don't do that, then.

 - if it's so widely used that you find it all over, it's probably
exported. grep for 'EXPORT.*fnname' to see where it is defined.

   We used to (brokenly) export things separately from the definition.
If you find cases of that, let's fix them.

Of course, usually I know roughly where it is defined, so I just limit
the pathnames for 'git grep'.

But the 'add the type of the return value or first argument to the
pattern' is often my second go-to (particularly for the cases where
you might be looking for _multiple_ definitions because it's some
architecture-specific thing, or you have some partial pattern because
every filesystem does their own thing).

Other 'git grep' patterns that often work for kernel sources:

 - looking for a structure declaration? Use

      git grep 'struct name {'

   which mostly works, but obviously depends on coding style so it's
not guaranteed. Good first thing to try, though.

 - use

        git grep '\t\.name\>.*='

   to find things like particular inode operations.

That second case is because we have almost universally converted our
filesystem operation initializers to use that named format (and really
strive to have a policy of constant structures of function pointers
only), and it's really convenient if you are doing VFS changes and
need to find all the places that use a particular VFS interface (eg
".get_acl" or similar).

It used to be a nightmare to find those things back when most of our
initializers were using the traditional unnamed ordered structure
initializers, so this is one area where we've introduced coding style
policies to make it really easy to grep for things (but also much
easier to add new fields and not have to add pointless NULL
initializer elements, of course).

             Linus

  reply	other threads:[~2021-09-26 21:04 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-10  3:09 incoming Andrew Morton
2021-09-10  3:10 ` [patch 1/9] mm: move kvmalloc-related functions to slab.h Andrew Morton
2021-09-10  3:10 ` [patch 2/9] rapidio: avoid bogus __alloc_size warning Andrew Morton
2021-09-10  3:10 ` [patch 3/9] Compiler Attributes: add __alloc_size() for better bounds checking Andrew Morton
2021-09-10  3:10 ` [patch 4/9] checkpatch: add __alloc_size() to known $Attribute Andrew Morton
2021-09-10  3:10 ` [patch 5/9] slab: clean up function declarations Andrew Morton
2021-09-10  3:10 ` [patch 6/9] slab: add __alloc_size attributes for better bounds checking Andrew Morton
2021-09-10  3:10 ` [patch 7/9] mm/page_alloc: " Andrew Morton
2021-09-10  3:10 ` [patch 8/9] percpu: " Andrew Morton
2021-09-10  3:10 ` [patch 9/9] mm/vmalloc: " Andrew Morton
2021-09-10 17:23   ` Linus Torvalds
2021-09-10 18:43     ` Kees Cook
2021-09-10 19:17       ` Linus Torvalds
2021-09-10 19:32         ` Kees Cook
2021-09-10 19:49     ` Nick Desaulniers
2021-09-10 20:16       ` Linus Torvalds
2021-09-10 20:47         ` Kees Cook
2021-09-10 20:58           ` Nick Desaulniers
2021-09-10 21:07             ` Kees Cook
2021-09-11  5:29     ` Joe Perches
2021-09-21 23:37     ` Kees Cook
2021-09-21 23:45       ` Joe Perches
2021-09-22  2:25         ` function prototype element ordering Kees Cook
2021-09-22  4:24           ` Joe Perches
2021-09-24 19:43             ` Kees Cook
2021-09-22  7:24           ` Alexey Dobriyan
2021-09-22  8:51             ` Joe Perches
2021-09-22 10:45               ` Alexey Dobriyan
2021-09-22 11:19             ` Jani Nikula
2021-09-22 21:15             ` Linus Torvalds
2021-09-23  5:10               ` Joe Perches
2021-09-25 19:40               ` David Laight
2021-09-26 21:03                 ` Linus Torvalds [this message]
2021-09-27  8:21                   ` David Laight
2021-09-27  9:22                     ` Willy Tarreau
2021-09-10 17:11 ` incoming Kees Cook
2021-09-10 20:13   ` incoming Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wjoLAYG446ZNHfg=GhjSY6nFmuB_wA8fYd5iLBNXjo9Bw@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=David.Laight@aculab.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=apw@canonical.com \
    --cc=cl@linux.com \
    --cc=danielmicay@gmail.com \
    --cc=dennis@kernel.org \
    --cc=dwaipayanray1@gmail.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=joe@perches.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lukas.bulwahn@gmail.com \
    --cc=mm-commits@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=ojeda@kernel.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=tj@kernel.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).