On Thu, 12 Jul 2007 08:57:51 -0500 Dave Kleikamp <shaggy@xxxxxxxxxxxxxxxxxx> wrote:
> On Thu, 2007-07-12 at 12:38 +0100, Andy Whitcroft wrote:
> > Andrew Morton wrote:
> > >> + 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.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
