diff mbox

[v4] ext3, ext4: make i signed in do_split()

Message ID 4935A21A.60509@gmail.com
State Accepted, archived
Headers show

Commit Message

roel kluin Dec. 2, 2008, 9:01 p.m. UTC 
Andrew Morton wrote:
> On Sat, 29 Nov 2008 04:40:42 -0500
> roel kluin <roel.kluin@gmail.com> wrote:
> 
>> unsigned i >= 0 is always true

>> -	for (i = count-1; i >= 0; i--) {
>> +	for (i = count-1; i < count; i--) {
			break;

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

You are right. although count is unsigned, dx_make_map() returns int, which is
assigned to count, so this should fit in an int. (If you ack this, make sure to
get this patch, and not one of the others.)
--------------->8----------------8<--------------------
Make i signed, otherwise the loop will not end when it becomes negative.

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

Theodore Ts'o Dec. 5, 2008, 4:32 p.m. UTC | #1 
On Tue, Dec 02, 2008 at 10:01:14PM +0100, Roel Kluin wrote:
> Andrew Morton wrote:
> > On Sat, 29 Nov 2008 04:40:42 -0500
> > roel kluin <roel.kluin@gmail.com> wrote:
> > 
> >> unsigned i >= 0 is always true
> 
> >> -	for (i = count-1; i >= 0; i--) {
> >> +	for (i = count-1; i < count; i--) {
> 			break;
> 
> > A local variable called `i' should always have signed type.  In fact,
> > it should have `int' type.
> 
> You are right. although count is unsigned, dx_make_map() returns int, which is
> assigned to count, so this should fit in an int. (If you ack this, make sure to
> get this patch, and not one of the others.)

ACK.  I'll take care of getting this into the patch queue.  I'll
include both the ext3 and ext4 variants, since this one is simple
enough that it's highly unlikely to conflict with anything Andrew
might have in the -mm tree.  This is also much more of a cleanup than
anything else, so I don't think we need to push it until the merge
window.  (I *think* it might be possible for a 2k filesystems and
*very* long filenames we might possibly be able to trigger a
failure but it would be very hard to do, and it's been around for a
very long time without anyone complaining about an actual real-life
problem getting trigger.)

						- Ted
--
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..4846327 100644
--- a/fs/ext3/namei.c
+++ b/fs/ext3/namei.c
@@ -1156,9 +1156,9 @@  static struct ext3_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
 	u32 hash2;
 	struct dx_map_entry *map;
 	char *data1 = (*bh)->b_data, *data2;
-	unsigned split, move, size, i;
+	unsigned split, move, size;
 	struct ext3_dir_entry_2 *de = NULL, *de2;
-	int	err = 0;
+	int	err = 0, i;
 
 	bh2 = ext3_append (handle, dir, &newblock, &err);
 	if (!(bh2)) {
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 63adcb7..bca90c1 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1166,9 +1166,9 @@  static struct ext4_dir_entry_2 *do_split(handle_t *handle, struct inode *dir,
 	u32 hash2;
 	struct dx_map_entry *map;
 	char *data1 = (*bh)->b_data, *data2;
-	unsigned split, move, size, i;
+	unsigned split, move, size;
 	struct ext4_dir_entry_2 *de = NULL, *de2;
-	int	err = 0;
+	int	err = 0, i;
 
 	bh2 = ext4_append (handle, dir, &newblock, &err);
 	if (!(bh2)) {