diff mbox

ext3: also fix loop in do_split()

Message ID 49310E1A.60105@gmail.com
State Accepted, archived
Headers show

Commit Message

roel kluin Nov. 29, 2008, 9:40 a.m. UTC
unsigned i >= 0 is always true

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---

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

Comments

Andrew Morton Dec. 2, 2008, 8:07 p.m. UTC | #1
On Sat, 29 Nov 2008 04:40:42 -0500
roel kluin <roel.kluin@gmail.com> wrote:

> unsigned i >= 0 is always true
> 
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> index 3e5edc9..8f5e15d 100644
> --- a/fs/ext3/namei.c
> +++ b/fs/ext3/namei.c
> @@ -1188,7 +1188,7 @@ static struct ext3_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
>  	/* Split the existing block in the middle, size-wise */
>  	size = 0;
>  	move = 0;
> -	for (i = count-1; i >= 0; i--) {
> +	for (i = count-1; i < count; i--) {
>  		/* is more than half of this entry in 2nd half of the block? */
>  		if (size + map[i].size/2 > blocksize/2)
>  			break;

A local variable called `i' should always have signed type.  In fact,
it should have `int' type.  Doing

	unsigned i;

is an act of insane vandalism, punishable by spending five additional
years coding in fortran.

I suggest you fix this by giving `i' the type God intended, or by
making it unsigned and then renaming it to something which is not
intended to trick programmers and reviewers.

Sheesh.
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen Dec. 2, 2008, 8:29 p.m. UTC | #2
Andrew Morton wrote:

> A local variable called `i' should always have signed type.  In fact,
> it should have `int' type.  Doing
> 
> 	unsigned i;
> 
> is an act of insane vandalism, punishable by spending five additional
> years coding in fortran.
> 
> I suggest you fix this by giving `i' the type God intended, or by
> making it unsigned and then renaming it to something which is not
> intended to trick programmers and reviewers.
> 
> Sheesh.

/me hangs head in shame, and points feebly but only halfheartedly at the
other people who reviewed the change when it originally went in... I
have no idea what I was thinking.  Sorry.  Please don't make me go back
to Fortran.

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

Patch

diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
index 3e5edc9..8f5e15d 100644
--- a/fs/ext3/namei.c
+++ b/fs/ext3/namei.c
@@ -1188,7 +1188,7 @@  static struct ext3_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
 	/* Split the existing block in the middle, size-wise */
 	size = 0;
 	move = 0;
-	for (i = count-1; i >= 0; i--) {
+	for (i = count-1; i < count; i--) {
 		/* is more than half of this entry in 2nd half of the block? */
 		if (size + map[i].size/2 > blocksize/2)
 			break;