===

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

+			# ...
[ 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.  But un...
[ message continues ]

For reference, here's Jan's proposal for Documentation/CodingStyle:

http://lkml.org/lkml/2008/2/26/462
Benny
--

Yes, seems reasonably well worded.  However, I see no consensus for its

acceptance as a change.  I seem some near NAK's.
-apw

--

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

--

Sorry.  I'm not going to change perfectly working editing habits *or* to patch

nvi to satisfy an annoying wunch of bankers.  HAND, GAFL.

--

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
--

"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...

--

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
--

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

--

We have these people too. And by large, I find spell fixes

better than oh-I-decided-to-run-checkpatch-on-existing-code bombs.

--

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

--

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/

--

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
--

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

time ago:
http://article.gmane.org/gmane.linux.kernel/644306
-Andi

--

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

--

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

--

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
--

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 submission.

Oth...
[ message continues ]

---

~Randy

--

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! :)

--

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

--

Dunno about others, but when I see that idiom I fall over stunned and

can't get up again.

--

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

--

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....
I am going to be off next week, but will likely be dipping into email

now and again.
-apw

--

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

--

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
--

And for other kernel developers tabs are for indention, spaces for alignment.
Forget it - the world will not unify about this.
	Sam

--

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.
--

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.

--

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
--

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.
--

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

--