===
total: 0 errors, 0 warnings, 36 lines checked

d27a9f5.diff has no obvious style problems and is ready for submission.
===
commit d27a9f5760fa231ab888f96e27355a001c88b239
Author: Jan Engelhardt <jengelh@computergmbh.de>
Date:   Sun Apr 6 06:49:01 2008 +0200

     checkpatch: relax spacing and line length

     We all had the arguments about 80 columns, so here goes a relax.
     Checking for 95 (or perhaps something better?), but of course we
     print "80" in the output, because if you happened to get to 95, it's
     "really time" to break it.

     This also relaxes the tab doctrine, because spaces DO make sense --
     especially when you view the code with a tab setting of not-8.

     Signed-off-by: Jan Engelhardt <jengelh@computergmbh.de>
---
  scripts/checkpatch.pl |   22 ++++++++++++++++------
  1 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 58a9494..e5a96c1 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1094,8 +1094,8 @@ sub process {
  			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
  			ERROR("trailing whitespace\n" . $herevet);
  		}
-#80 column limit
-		if ($line =~ /^\+/ && !($prevrawline=~/\/\*\*/) && $length > 80) {
+#80 column limit (yes, testing for 95 is correct, we do not want to annoy)
+		if ($line =~ /^\+/ && !($prevrawline=~/\/\*\*/) && $length >= 95) {
  			WARN("line over 80 characters\n" . $herecurr);
  		}

@@ -1107,12 +1107,22 @@ sub process {
  # check we are in a valid source file *.[hc] if not then ignore this hunk
  		next if ($realfile !~ /\.[hc]$/);

+		my $arg = qr/$Type(?: ?$Ident)?/;
+		if ($rawline =~ /^\+ +($arg, )*$arg(?:,|\);?)$/) {
+			# We are probably in a function parameter list.
+			# Spaces ok -- used for align.
+		} elsif ($rawline =~ /^\+\t+ *\S/) {
+			# We are probably in a function or an array/struct
+			# definition/initializer. Spaces ok -- used for align
+			# on multiline statements.
+		} else {
  # at ...
[ message continues ]

This will reduce the usefulness of checkpatch for those developers who

Non-tab-using code inevitably ends up having a mix of tabs and non-tabs
and looks a mess if tabstops are set to anything other than eight.

God I wish I had not been cc'ed on this.
--

[ message continues ]

I humbly disagree.  If you use tabs for (logical) indentation and then trailing
spaces for (graphical) alignment, not just a random mix of tabs and spaces the
code looks fine with 8-character wide tabs or any other setup.  Only when you
use tabs for graphical alignment, below fixed-width characters on the line above
you marry yourself with a specific tab expansion width.


The idea is *not* to open the flood gates, just to allow for a bit more

--

[ message continues ]

Isn't this an editor/tools issue more than a checkpatch one?

I think you should first try to get the most frequently used
code editors and pretty printers (vim/emacs/eclipse/indent, etc)
to support the tabs to level, spaces to align style.

g'luck with that.

--

[ message continues ]

I have one such editor to date, Me. But I can not use it's output
because checkpatch.pl forbids it, and I get scorned by my maintainers.
So I think you got it reversed. First allow it then automate for it
other wise there is no use, right?

Boaz
--

[ message continues ]

No, it's really time to teach people that checkpatch is *not* a tool for 
janitors.

It's a tool for patch submitters and maintainers that automates a part 
of patch review.

When a patch has a few checkpatch warnings and the submitter can justify 
them (since the code would otherwise look bad) *that is OK*.

But if your driver has over 2000 lines over 80 columns or many lines 
over 95 columns it is not "really time to break it" - it is time to 

In the kernel all tabs are 8 spaces wide.

When you view the code with a different setting that's your fault.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

--

[ message continues ]

And for other kernel developers tabs are for indention, spaces for alignment.

Forget it - the world will not unify about this.

	Sam
--

[ message continues ]

Resistance to change is only natural, but OTOH no change leads to stagnation.
I completely agree that if we don't encourage this as an alternative it will
indeed never happen but just saying "forget it" brings us nowhere.

I still believe that once people get used to this mentally they can see this
method's merits and how its logic relates to the program's structure: syntax
and coding style.


--

[ message continues ]

Why is it better for the end of the line to sometimes go a bit off the
right of the screen, but not too far?  Are you trying to stop checkpatch
whinging about lines which simply cannot be split pleasently and so poke
a little off, or are you keen to have a bit more space to write code in
as a general rule?

If its the former then you are missing the point of checkpatch, it is
mearly an advisor trying to point those things you have done which the
maintainer is very likely going to notice and going to have to deal with
either by rejecting your patch or fixing it themselves; it is a time
saving aid to them and you.  If the statement really cannot be sanely
wrapped somewhere then do not wrap it.  The maintainer should be able to
see you are correct, if they dissagree they will re-wrap it where they
think it can be sanely wrapped.  While I can imagine that you might have
one or two difficult wraps in a large set of patches, I would not expect
the number of false reports to be significant.  I personally have seen
three or four a year in all the patches I have produced and reviewed.
So it does not seem worth changing the underlying rule just to avoid these
easily ignored reports, expecially considering the number of genuinely
bad lines it would then pass.

If its the latter (you want more space generally), then we should just
say "line length is now N" and be done with it, 95, 128, 200 whatever.
Letting you have 95 characters before telling you should not have had 81

This is about visually representing the tabs as smaller units, and still
wanting the rest of the code to line up correctly.  Although I can see
that the effect is somewhat desirable, it feels very much like doing
just this will then go on to encourage the writer to want to violate the
overall line length (as you have more space) and lead to the need to have
more characters on a line in the first place.

For myself I would not necessarily have a problem with this, as I should
be unable to see it in my tabs=8 world.  ...
[ message continues ]

Here was a concrete proposal (with patch for CodingStyle) from my side some 
time ago:

http://article.gmane.org/gmane.linux.kernel/644306

-Andi
--

[ message continues ]

Yeah, that was a good example of how to go about changinging things.  As
I said on that thread I would happily change checkpatch to follow suit.
Oddly for a CodingStyle discussion there was very "discussion" at all.
It feels like people are scared of the passion that often errupts in
discussions over style.

On that particular suggestion, I would probabally be in favour, and
slightly happier to have the string on the printk line for consistency
even though that would violate the "don't hide information" rule.

-apw
--

[ message continues ]

Oh, and if people felt that the concensus was for something to be
implemented and that you are waiting for me to implement the change in
checkpatch; please say so.

-apw
--

[ message continues ]

Well at least I think the printk change is a good one to implement and there 
wasn't much protest to it at least.

If you're looking for another change request I think dropping the trailing
white space check would be good a idea because a lot of maintainers already
handle that automatically and it is not really worth bothering the "leaf
developers" with because that can be trivially automated.

Also I still think --file needs to go, or at least be replaced with
--file-force and warning. See the recent unpleasant incident it caused
again, e.g.

http://thread.gmane.org/gmane.linux.kernel/656847/focus=657003
and 
http://thread.gmane.org/gmane.linux.kernel/656847/focus=657003

-Andi

--

[ message continues ]

I assume you are talking about git squashing bad line ends as it loads
the patches up.  I thought that this still triggered issues when loading
subsequent patches which relied on the context including those spaces.
Surely, this would lead to increased load on the maintainers as they
loaded those patches up?  Where the leaf maintainers use quilt would they
either not get seen or trigger failurs for him (dependant on whether he
has the option enabled), both bad outcomes.  Better to waste the time of

Ok, these are both the same link, so either you miss pasted one, or you
repeat to emphasise.

Reading over that thread there seems to be a few themes:

1) "don't send out 150 patches at once",
2) accepting checkpatch only changes is stupid, and
3) some maintainers do not like the style checkpatch recommends.

The first is really a general complaint against the originator, not really
a complaint about checkpatch.  Checkpatch may have prompted the changes but
that is not ultimatly its fault.  We may not like a huge pile of patches
dropped at once, but failure to recognise that is a failing in education,
not a tooling issue.  Also with the huge volumes on linux-kernel these
are hardly a major component of overall volumes?

The second is actually a matter for maintainers, either they will accept
checkpatch only changes or they won't.  Clearly Dave Miller and Andi
will not, clearly Ingo will.  Surly that is a matter for the Maintainer,
and not for the tool.

The third is about some maintainers wishing to use different style for
their parts of their tree.  That is their right, assuming those upstream
of them will accept their style.


There is a lot of polorised view on checkpatch.  But clearly there are
maintainers on both sides of the argument, those who find the tool
helps them and those who do not.  Maintainers who believe that they
should police the coding style, and find that the tool helps them with
that part of the process reducing their burden when reviewing ...
[ message continues ]

Instead of

	if (foo) {
		if (baz) {
			++x;
printk("Oh so long line makes my coding style go wary... nonsensical sentence\n");
		}
	}

I'd keep the indent and allow elongated lines:

	if (foo) {
		if (baz) {
			++x;
			printk("Oh so long line makes my coding style go wary... nonsensical sentence\n");
		}
	}

Or perhaps you just pointed out we need a smarter grep program! :)
--

[ message continues ]

My preference would be for the latter.  Keep the line indent consistent
and allow the line to overspill.  But that would depend on the concensus
obviously.  The originally suggested layout was:

			printk(
"Oh ....",
				a, b);

-apw
--

[ message continues ]

Dunno about others, but when I see that idiom I fall over stunned and 
can't get up again.
--

[ message continues ]

Not sure what the problem is. But my original proposal had two alternatives.

Either unindent (as above) or exceed 80 characters for the format string as 
needed to keep it together. 

I've used both in the past and don't have a particular preference.

-Andi
--

[ message continues ]

That remains me of nethack.  I'll start with the width breaker.

-apw
--

[ message continues ]

Ok.  I've just pushed out a testing version of checkpatch which is
identicle the 0.18 delta I just pushed to Andrew with the printk width
restriction lifted.  Perhaps those interested might test it to see if it
whines appropriatly:

  http://www.kernel.org/pub/linux/kernel/people/apw/checkpatch/checkpatch.pl-testing

I am going to be off next week, but will likely be dipping into email
now and again.

-apw
--

[ message continues ]

---
~Randy
--

[ message continues ]

As I said on a different thread, tabs should still be accounted for as
8 spaces for checking the total line length.  The motivation is to



It'd be great if we could verify that the number of tabs is equal or
greater than the nesting level (and then followed by zero or more spaces).

This brings into mind if we could/should warn about excessive nesting?
I think it might be a good idea, though I'm not sure what would be

--

[ message continues ]

For reference, here's Jan's proposal for Documentation/CodingStyle:
http://lkml.org/lkml/2008/2/26/462

Benny

--

[ message continues ]

Yes, seems reasonably well worded.  However, I see no consensus for its
acceptance as a change.  I seem some near NAK's.

-apw
--

[ message continues ]

Seriously, I'm not sure how significant or relevant they are though.

In http://lkml.org/lkml/2008/2/26/533,

I'm not sure what "two space change" proposal this Steve referred to
and his rejection is based on not-to-sound aesthetic grounds.

The motivation behind our proposal is more than just aesthetic.
I believe that using tabs for indent and then spaces for alignment
is functionally better, works for everybody, and will eventually result
in a more readable code over time, hopefully leading to fewer bugs.


Randy's answer, http://lkml.org/lkml/2008/2/27/7

My interpretation of that is the the current CodingStyle is too detailed
*now* therefore we need to relax it, not keep it the way it is.
It's true, that we add more details to relax the requirements but
overall we'd allow for more flexibility.  To do that with removing
details rather than adding any is dangerous IMO since it can easily
lead to indentation chaos that makes everybody's life harder...

Richard Knutsson, in http://lkml.org/lkml/2008/2/28/356
adds an excellent point about needing smaller tab expansion
for narrow screens.


Again, I see no real reasons why not to besides being against Stefan's
preferences.  I repeat my point that the proposed style does not
necessarily encourage smaller tab expansion, it just makes it possible.

Well, enough said.
Back to fixin' bugs...

Benny
--

[ message continues ]

BTW, my preference was about keeping the last traces of witty language
in this text, not about any particular whitespace language.

(Do you have an idea who wrote the sentences which that patch wanted to
delete, and more importantly, *why* he wrote it this way?  You
apparently don't yet, but maybe you think about it once more.  Thanks.)
-- 
Stefan Richter
-=====-==--- -=-- -=--=
http://arcgraph.de/sr/
--

[ message continues ]

I don't see it as really anything other than different.  It'll look better
for you, sure.  But either we (tabs=8 people) will not be maintaining it
as we edit leading to inconsistent indent, or we will be putting in lots

But that is your option.  Right now the rule is simple, use tabs, only.
Either we relax that and get inconsistent indent from the two camps _or_

Indeed.

-apw
--

[ message continues ]

Tabs + a variable number spaces to match up logically with the
previous line is O.K.  Tabs + exactly two spaces is what I objected
to.  I only posted due to being the originator of the Linux Kernel Emacs
cc-mode style and cc-mode works like the former.

For whatever it's worth, my sentiments are on the David Miller side,
having been the lucky recipient of format changes only patch bombs
before.  I suppose you should be glad you don't have someone running
a spell checker on the entire source code.

-sb
--

[ message continues ]

We have these people too. And by large, I find spell fixes
better than oh-I-decided-to-run-checkpatch-on-existing-code bombs.
--

[ message continues ]

Sorry.  I'm not going to change perfectly working editing habits *or* to patch
nvi to satisfy an annoying wunch of bankers.  HAND, GAFL.
--

[ message continues ]

Thanks for the insightful and mature comment, Al.

I really hate to spend more time on this topic but folks did find merits in it.
There's no need to change anybody's editing habits if we allow this indentation
style in the CodingStyle document and in checkpatch.pl in addition to the
existing convention.

Benny

--

[ message continues ]

"Allow" is such a nice word, isn't it?  Let's take a closer look:
	* nobody prohibits lines satisfying your constraints ("tabs only for
indent level"), so "allowing" that is meaningless
	* "indentation style" in the above refers to editor settings.
To "allow" that, you advocate prohibiting the lines _NOT_ satisfying your
constraints.  Which, by definition, means extra work for people submitting
patches, no matter how you spin it.

BTW, while we are talking about conventions, would you mind keeping lines
in your mail shorter than 79 columns to avoid wraparounds in quoted text?
Unlike your proposal, that one actually _is_ a common convention...
--

[ message continues ]

Currently checkpatch.pl prints an error if I use 8 or more spaces in the
indentation string and Documentation/CodingStyle says:

"Outside of comments, documentation and except in Kconfig, spaces are never
used for indentation"

Although CodingStyle and checkpatch just provide guidance and the final
word is the maintainer's I consider these recommendations as "disallowing",
or at least "discouraging". So did others that commented on patches I sent
in the past.  If that wasn't the case I wouldn't have come up with this

I'm certainly not advocating to prohibit the current indentation style,
just to relax the rules to allow a superset of it.

Basically, I'd like checkpatch to allow /^\+\t* *\S/
and, since Andy says that checkpatch knows "the indent to some degree",

No, I don't mind.

[Though it is a bit of a pain to keep that when automatic wrapping of long
lines is turned off in my mail program so I can easily quote patches or
code snippets.]

Benny

--

[ message continues ]