linux-ext4 - Re: [EXT4 set 2][PATCH 2/5] cleanups: Add extent sanity checks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1184248671.10312.7.camel@kleikamp.austin.ibm.com>
Date:	Thu, 12 Jul 2007 08:57:51 -0500
From:	Dave Kleikamp <shaggy@...ux.vnet.ibm.com>
To:	Andy Whitcroft <apw@...dowen.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>, cmm@...ibm.com,
	linux-kernel@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [EXT4 set 2][PATCH 2/5] cleanups: Add extent sanity checks

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.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html