ext2 cpu<->le32 thinkos? (-test10)

PATCH In review Marko Kreen 2 participants 2 messages Nov 8, 2000

lore

Share & tools

lore

Marko Kreen

author

2000-11-08 16:54 UTC #0 lore raw

PATCH 1/1 ext2 cpu<->le32 thinkos? (-test10)

Reading ext2 code I found some inconsistencies
in endianess handling, could anyone comment
on this?

Also there is a unnecessary RDONLY check.

I understand that it happens to work anyway
but they confuse understanding.

Or am I missing something?

Signature · 65 lines

marko



diff -urNX /home/marko/misc/diff-exclude fs/ext2.orig/ialloc.c fs/ext2/ialloc.c
--- fs/ext2.orig/ialloc.c	Fri Oct  6 22:49:00 2000
+++ fs/ext2/ialloc.c	Wed Nov  8 10:09:23 2000
@@ -399,11 +399,6 @@
 			ext2_error (sb, "ext2_new_inode",
 				    "Free inodes count corrupted in group %d",
 				    i);
-			if (sb->s_flags & MS_RDONLY) {
-				unlock_super (sb);
-				iput (inode);
-				return NULL;
-			}
 			gdp->bg_free_inodes_count = 0;
 			mark_buffer_dirty(bh2);
 		}
diff -urNX /home/marko/misc/diff-exclude fs/ext2.orig/namei.c fs/ext2/namei.c
--- fs/ext2.orig/namei.c	Wed Nov  1 20:31:30 2000
+++ fs/ext2/namei.c	Wed Nov  8 13:46:20 2000
@@ -242,7 +242,7 @@
 
 				de = (struct ext2_dir_entry_2 *) bh->b_data;
 				de->inode = 0;
-				de->rec_len = le16_to_cpu(sb->s_blocksize);
+				de->rec_len = cpu_to_le16(sb->s_blocksize);
 				dir->i_size = offset + sb->s_blocksize;
 				dir->u.ext2_i.i_flags &= ~EXT2_BTREE_FL;
 				mark_inode_dirty(dir);
@@ -750,7 +750,7 @@
 		if (retval)
 			goto end_rename;
 	} else {
-		new_de->inode = le32_to_cpu(old_inode->i_ino);
+		new_de->inode = cpu_to_le32(old_inode->i_ino);
 		if (EXT2_HAS_INCOMPAT_FEATURE(new_dir->i_sb,
 					      EXT2_FEATURE_INCOMPAT_FILETYPE))
 			new_de->file_type = old_de->file_type;
@@ -785,7 +785,7 @@
 	old_dir->u.ext2_i.i_flags &= ~EXT2_BTREE_FL;
 	mark_inode_dirty(old_dir);
 	if (dir_bh) {
-		PARENT_INO(dir_bh->b_data) = le32_to_cpu(new_dir->i_ino);
+		PARENT_INO(dir_bh->b_data) = cpu_to_le32(new_dir->i_ino);
 		mark_buffer_dirty(dir_bh);
 		old_dir->i_nlink--;
 		mark_inode_dirty(old_dir);
diff -urNX /home/marko/misc/diff-exclude fs/ext2.orig/super.c fs/ext2/super.c
--- fs/ext2.orig/super.c	Fri Oct  6 22:49:00 2000
+++ fs/ext2/super.c	Wed Nov  8 10:08:22 2000
@@ -101,7 +101,7 @@
 	int i;
 
 	if (!(sb->s_flags & MS_RDONLY)) {
-		sb->u.ext2_sb.s_es->s_state = le16_to_cpu(sb->u.ext2_sb.s_mount_state);
+		sb->u.ext2_sb.s_es->s_state = cpu_to_le16(sb->u.ext2_sb.s_mount_state);
 		mark_buffer_dirty(sb->u.ext2_sb.s_sbh);
 	}
 	db_count = sb->u.ext2_sb.s_db_per_group;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Andreas Dilger

2000-11-08 17:56 UTC #1 lore raw

Quoted 3 lines · Marko

Reading ext2 code I found some inconsistencies
in endianess handling, could anyone comment
on this?

Your fixes look OK to me. For all architectures the two operations the two operations are interchangeable, so it hasn't given us any problems. However, it is good to fix these sort of things to prevent headaches.

Quoted 1 line

Also there is a unnecessary RDONLY check.

Yes, you are correct. This is inside ext2_new_inode() which should never be called for a read-only filesystem in the first place. If it is, this check won't help us anyways since it is only inside an error path, and the code is much more broken than this.

Cheers, Andreas

Quoted 62 lines

diff -urNX /home/marko/misc/diff-exclude fs/ext2.orig/ialloc.c fs/ext2/ialloc.c

ext2/ialloc.c renamed from ext2.orig/ialloc.c +0 -5

@@ -399,11 +399,6 @@
 			ext2_error (sb, "ext2_new_inode",
 				    "Free inodes count corrupted in group %d",
 				    i);
-			if (sb->s_flags & MS_RDONLY) {
-				unlock_super (sb);
-				iput (inode);
-				return NULL;
-			}
 			gdp->bg_free_inodes_count = 0;
 			mark_buffer_dirty(bh2);
 		}

diff -urNX /home/marko/misc/diff-exclude fs/ext2.orig/namei.c fs/ext2/namei.c

ext2/namei.c renamed from ext2.orig/namei.c +3 -3

@@ -242,7 +242,7 @@
 
 				de = (struct ext2_dir_entry_2 *) bh->b_data;
 				de->inode = 0;
-				de->rec_len = le16_to_cpu(sb->s_blocksize);
+				de->rec_len = cpu_to_le16(sb->s_blocksize);
 				dir->i_size = offset + sb->s_blocksize;
 				dir->u.ext2_i.i_flags &= ~EXT2_BTREE_FL;
 				mark_inode_dirty(dir);

@@ -750,7 +750,7 @@
 		if (retval)
 			goto end_rename;
 	} else {
-		new_de->inode = le32_to_cpu(old_inode->i_ino);
+		new_de->inode = cpu_to_le32(old_inode->i_ino);
 		if (EXT2_HAS_INCOMPAT_FEATURE(new_dir->i_sb,
 					      EXT2_FEATURE_INCOMPAT_FILETYPE))
 			new_de->file_type = old_de->file_type;

@@ -785,7 +785,7 @@
 	old_dir->u.ext2_i.i_flags &= ~EXT2_BTREE_FL;
 	mark_inode_dirty(old_dir);
 	if (dir_bh) {
-		PARENT_INO(dir_bh->b_data) = le32_to_cpu(new_dir->i_ino);
+		PARENT_INO(dir_bh->b_data) = cpu_to_le32(new_dir->i_ino);
 		mark_buffer_dirty(dir_bh);
 		old_dir->i_nlink--;
 		mark_inode_dirty(old_dir);

diff -urNX /home/marko/misc/diff-exclude fs/ext2.orig/super.c fs/ext2/super.c

ext2/super.c renamed from ext2.orig/super.c +1 -2

@@ -101,7 +101,7 @@
 	int i;
 
 	if (!(sb->s_flags & MS_RDONLY)) {
-		sb->u.ext2_sb.s_es->s_state = le16_to_cpu(sb->u.ext2_sb.s_mount_state);
+		sb->u.ext2_sb.s_es->s_state = cpu_to_le16(sb->u.ext2_sb.s_mount_state);
 		mark_buffer_dirty(sb->u.ext2_sb.s_sbh);
 	}
 	db_count = sb->u.ext2_sb.s_db_per_group;
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/

Signature · 7 lines

Andreas Dilger  \ "If a man ate a pound of pasta and a pound of antipasto,
                 \  would they cancel out, leaving him still hungry?"
http://www-mddsp.enel.ucalgary.ca/People/adilger/               -- Dogbert
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
Please read the FAQ at http://www.tux.org/lkml/