Kernel Newbies archive on lore.kernel.org
 help / color / Atom feed
From: Greg KH <greg@kroah.com>
To: Konstantin Andreev <andreev@swemel.ru>
Cc: kernelnewbies@kernelnewbies.org
Subject: Re: linux kernel coding style and checkpatch.pl script
Date: Thu, 26 Mar 2020 14:17:32 +0100
Message-ID: <20200326131732.GA1296483@kroah.com> (raw)
In-Reply-To: <E1jHS2D-0004t2-IX@shelob.surriel.com>

On Thu, Mar 26, 2020 at 04:01:18PM +0300, Konstantin Andreev wrote:
> Valdis Klētnieks, 26 Mar 2020 07:13 MSK:
> > Don't split literal strings, it means that grepping the source tree for "already registered" fails. Making grep for a string work is more important than shutting up checkpatch.
> 
> On Thu, Mar 26, 2020 at 02:36:23PM +0300, Konstantin Andreev wrote:
> > Sic! Grepping is important. Given that, why are kernel functions coded in a
> > 
> > | static int __init loglevel(char *str)
> > | {
> > 
> > way, but not old decent unix way:
> > 
> > | static int __init
> > | loglevel(char *str)
> > | {
> 
> Greg KH, 26 Mar 2020 15:06 MSK:
> > Documentation/process/coding-style.rst
> 
> This document does not answer my question. It does not even require
> 
> | static int __init loglevel(char *str)
> | {
> 
> style. Here is a relevant part of document: "separate functions with one blank line... EXPORT macro should follow closing function brace ... In function prototypes, include parameter names ... E.g.:"
> 
> | int system_is_up(void)
> | {
> |         return system_state == SYSTEM_RUNNING;
> | }
> | EXPORT_SYMBOL(system_is_up);
> 
> That's all. Have I overlooked something? Could you, please, share your own point of view?

If it's not a specific rule in there, then that means you could do
someething crazy like:
	static int __init
	loglevel(char *str)
	{
if you were so loony to do so.

But, it turns out that when you write your code like that, it's harder
to actually find where the function is defined vs. where it is called
when greping a codebase.

That's why those of us who have been at this for a long time prefer:
	static int __init loglevel(char *str)
	{
instead.

much easier to pick out of a list of "where is this defined" output of
'git grep'.

And yes, I know all about ctags and the like, but sometimes you don't
have access to that, or don't want to fire it up and have it pre-process
things just to look at a specific git tree at the moment.

Also, big shoutout to 'vgrep' https://github.com/vrothberg/vgrep which
makes using 'git grep' a zillion times easier and is what I rely on all
the time.

tldr;
	either way is fine, but putting the return value on the function
	name line usually makes it easier for others to find your code.

Hope this helps,

greg k-h

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies

  parent reply index

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-25  9:36 Tomek The Messenger
2020-03-25  9:51 ` Greg KH
2020-03-26  4:13 ` Valdis Klētnieks
2020-03-26 11:36   ` Konstantin Andreev
     [not found]   ` <E1jHQrV-0002Zv-Ln@shelob.surriel.com>
2020-03-26 12:06     ` Greg KH
2020-03-26 13:01       ` Konstantin Andreev
     [not found]       ` <E1jHS2D-0004t2-IX@shelob.surriel.com>
2020-03-26 13:17         ` Greg KH [this message]
2020-03-26 10:57 ` Anuz Pratap Singh Tomar

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=20200326131732.GA1296483@kroah.com \
    --to=greg@kroah.com \
    --cc=andreev@swemel.ru \
    --cc=kernelnewbies@kernelnewbies.org \
    /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

Kernel Newbies archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kernelnewbies/0 kernelnewbies/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kernelnewbies kernelnewbies/ https://lore.kernel.org/kernelnewbies \
		kernelnewbies@kernelnewbies.org
	public-inbox-index kernelnewbies

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernelnewbies.kernelnewbies


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git