On Thu, 12 Jul 2007 08:57:51 -0500 Dave Kleikamp [off-list ref] wrote:

On Thu, 2007-07-12 at 12:38 +0100, Andy Whitcroft wrote:

quoted

Andrew Morton wrote:

quoted

quoted

+			if (ext4_ext_check_header(inode, ext_block_hdr(bh),
+							depth - i - 1)) {
+				err = -EIO;
+				break;
+			}
+			path[i+1].p_bh = bh;

Really that should have been "i + 1".  checkpatch misses this.  It seems to
be missing more stuff that it used to lately.

This one is difficult.  The rules up to now have been consistent spacing
is required on both sides of mathematics operators.  I personally like
spaces always, but we do tend to use them without spaces too where the
binding is effectivly part of the value -- the classic case is something
like:

	pfn << MAX_ORDER-1

In allowing that sort of thing, we implictly allow the one you note
above.  We have tried to be overly annoying on these things, and so the
check is consistancy, spaces both or neither.  We could be stricter.

I personally think stricter is better.  An occasionally false-positive
isn't going to hurt anyone.  (Well, maybe the checkpatch.pl maintainers
will get nagged.)  It at least will cause the developer to look at the
line of code in question and make a conscious decision to leave it as it
is.  I'm assuming that upstream maintainers use checkpatch.pl with some
constraint, and don't throw every patch that produces a warning back at
the submitter.

I'm in two minds.  Missing-the-spaces is pretty damn common and is sometimes
a reasonable way of saving quite a lot of horizontal space.  I spose we could
take it out again if it's causing problems.