From: Mark Lord <kernel@teksavvy.com>
To: Joe Perches <joe@perches.com>
Cc: Sergey Matyukevich <geomatsi@gmail.com>,
linux-kernel@vger.kernel.org, Jeff Garzik <jgarzik@pobox.com>,
linux-ide@vger.kernel.org
Subject: Re: [PATCH 02/10] drivers/ata: Fix continuation line formats
Date: Thu, 04 Feb 2010 09:07:46 -0500 [thread overview]
Message-ID: <4B6AD4B2.7070406@teksavvy.com> (raw)
In-Reply-To: <1265260575.31964.13.camel@Joe-Laptop.home>
Joe Perches wrote:
> If it's ever agreed that all lines > 80 cols are OK or
> new args after column 80 are OK, then sure. Until then,
> if you want it, you should do it.
..
In case you missed it, it *has* already been "agreed" that
there's nothing magical about 80 anymore.
-------- Original Message --------
Subject: Re: [PATCH] ptrace: checkpatch fixes
Date: Wed, 8 Apr 2009 10:19:36 -0700 (PDT)
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Roland McGrath <roland@redhat.com>
CC: Andrew Morton <akpm@linux-foundation.org>, linux-kernel@vger.kernel.org
References: <20090408062106.39EE0FC3E5@magilla.sf.frob.com>
On Tue, 7 Apr 2009, Roland McGrath wrote:
>
> This fixes all the checkpatch --file complaints about kernel/ptrace.c
> and also removes an unused #include. I've verified that there are no
> changes to the compiled code on x86_64.
Please don't bother with that insane "line length" option when using
"--file". At least not if the "fix" is to just mindlessly split the line.
That is _never_ a fix.
Changes like these:
> -int ptrace_readdata(struct task_struct *tsk, unsigned long src, char __user *dst, int len)
> +int ptrace_readdata(struct task_struct *tsk, unsigned long src,
> + char __user *dst, int len)
> -int ptrace_writedata(struct task_struct *tsk, char __user *src, unsigned long dst, int len)
> +int ptrace_writedata(struct task_struct *tsk, char __user *src,
> + unsigned long dst, int len)
> case PTRACE_GETEVENTMSG:
> - ret = put_user(child->ptrace_message, (unsigned long __user *) data);
> + ret = put_user(child->ptrace_message,
> + (unsigned long __user *) data);
just make the code harder to 'grep'.
Yes, at some point you have to split lines, but that point is not 80
columns any more. The advantage of getting the whole line when grepping
for function names much outweighs the downside of somebody using those
old 80x24 green phosphorous vt52's.
[ The same thing very much goes for complex if-statements etc. If
people can't stand the long lines, the primary solution would be to
turn a complex conditional into a helper inline functions, or to fix
excessive indentation by splitting up functions.
In the above case, the last one could perhaps have been handled
creating a new variable for and moving the cast to the initialiser,
for example. Is it worth it to avoid a 85-column line? Probably not.
And some lines just end up long. I think 100 characters may be a
more reasonable limit for "too long", but quite frankly, it depends on
the line.
So I think 'checkpatch' is pure crap in this area, and I've told
people so before, and they keep telling me that it has relaxed it's
idiotic warnings, but that is apparently just a lie. ]
Oh well. If I actually read perl, I could parse what the hell those
80-character rules are in checkpath. It already has random "it's ok if
X" stuff. But it never seems to really have any "oh, but splitting is
worse" logic.
Linus
next prev parent reply other threads:[~2010-02-04 14:07 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <cover.1265095093.git.joe@perches.com>
2010-02-02 7:22 ` [PATCH 01/10] arch/ia64: Fix continuation line formats Joe Perches
2010-02-07 11:43 ` Avi Kivity
2010-02-02 7:22 ` [PATCH 02/10] drivers/ata: " Joe Perches
2010-02-03 17:21 ` Mark Lord
2010-02-03 17:25 ` Jeff Garzik
2010-02-03 19:15 ` Joe Perches
2010-02-04 1:39 ` Krzysztof Halasa
2010-02-04 3:36 ` Joe Perches
2010-02-04 4:59 ` Mark Lord
2010-02-04 5:13 ` Andrew Morton
2010-02-04 14:03 ` Mark Lord
2010-02-04 5:16 ` Joe Perches
2010-02-04 14:07 ` Mark Lord [this message]
2010-02-02 7:22 ` [PATCH 03/10] drivers/gpu/drm/i915/intel_bios.c: " Joe Perches
2010-02-02 7:22 ` [PATCH 04/10] drivers/hwmon/vt8231.c: " Joe Perches
2010-02-02 7:56 ` [lm-sensors] " Jean Delvare
2010-02-02 8:01 ` Joe Perches
2010-02-02 7:22 ` [PATCH 05/10] drivers/net/davinci_emac.c: " Joe Perches
2010-02-02 7:22 ` [PATCH 06/10] drivers/scsi/qla2xxx/qua_os.c: " Joe Perches
2010-02-02 7:22 ` [PATCH 07/10] drivers/spi/spi_s3c64xx.c: " Joe Perches
2010-02-02 8:21 ` Grant Likely
2010-02-02 7:22 ` [PATCH 08/10] drivers/staging: " Joe Perches
2010-02-02 7:22 ` [PATCH 09/10] drivers/video/via: " Joe Perches
2010-02-03 22:36 ` Florian Tobias Schandinat
2010-02-02 7:22 ` [PATCH 10/10] sound/soc/s3c24xx/s2c-pcm.c: " Joe Perches
2010-02-02 11:40 ` Mark Brown
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=4B6AD4B2.7070406@teksavvy.com \
--to=kernel@teksavvy.com \
--cc=geomatsi@gmail.com \
--cc=jgarzik@pobox.com \
--cc=joe@perches.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-kernel@vger.kernel.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
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