Re: [PATCH 1/2] Make cramfs little endian only

[Andi Drebes <lists-receive@programmierforen.de>]

Jeff Garzik wrote:
> Bitfields also generate lower-quality assembly than masks&shifts 
> (typically more instructions using additional temporaries to accomplish 
> the same thing), based on my own informal gcc testing.
> 
> You would think gcc would be advanced enough to turn bitfield use into 
> masks and shifts under the hood, but for whatever reason that often is 
> not the case in kernel code.
OK. I'll modify the cramfs structures and replace the bitfields so that the
data can be accessed using masks and shifts. This requires updating the userspace
tools aswell. There will be a new patch avaliable soon.

I worked on the copy-and-convert-problem today and created a new patch that
avoids the negative things pointed out by Linus earlier. I hope I got that
in the right way. However, there are still some functions whose implementations
depend on the machine's endianness. I hope that they will be obsolete as soon as
the new patch is available.

The changes were tested on the following types of machines:
An i386 compatible box (little endian)
UltraSparc IIi (big endian)

Signed-off-by: Andi Drebes <andi@programmierforen.de>
---
 fs/cramfs/inode.c |  131 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 96 insertions(+), 35 deletions(-)

diff --git a/fs/cramfs/inode.c b/fs/cramfs/inode.c
index 350680f..ceb2c6e 100644
--- a/fs/cramfs/inode.c
+++ b/fs/cramfs/inode.c
@@ -4,6 +4,10 @@
  * Copyright (C) 1999 Linus Torvalds.
  *
  * This file is released under the GPL.
+ *
+ * Changelog:
+ *	11/07 - Andi Drebes <andi@programmierforen.de>
+ *	Made cramfs little endian only.
  */
 
 /*
@@ -37,9 +41,62 @@ static DEFINE_MUTEX(read_mutex);
 
 /* These two macros may change in future, to provide better st_ino
    semantics. */
-#define CRAMINO(x)	(((x)->offset && (x)->size)?(x)->offset<<2:1)
+#define CRAMINO(x)	((cramfs_offset_to_cpu(x) && cramfs_size_to_cpu(x)) ? \
+			 cramfs_offset_to_cpu(x) << 2 : 1)
 #define OFFSET(x)	((x)->i_ino)
 
+#ifdef __BIG_ENDIAN
+
+/* Converts a cramfs_inode's offset field
+   from little endianto cpu endian */
+static u32 cramfs_offset_to_cpu(struct cramfs_inode *inode)
+{
+	u8 *inode_bytes = (u8 *)inode;
+	return ((inode_bytes[8] & 0xc0) >> 6) | (inode_bytes[9] << 2) |
+		(inode_bytes[10] << 10) | (inode_bytes[11] << 18);
+}
+
+/* Converts a cramfs_inode's namelength field
+   from little endian to cpu endian */
+static u32 cramfs_namelen_to_cpu(struct cramfs_inode *inode)
+{
+	u8 *inode_bytes = (u8 *)inode;
+	return inode_bytes[8] & 0x3f;
+}
+
+/* Converts a cramfs_inode's size field
+   from little endian to cpu endian */
+static u32 cramfs_size_to_cpu(struct cramfs_inode *inode)
+{
+	return ((inode->size & 0xff0000) >> 16) | (inode->size & 0x00ff00) |
+		((inode->size & 0x0000ff) << 16);
+}
+
+#elif defined(__LITTLE_ENDIAN)
+
+/* Converts a cramfs_inode's offset field
+   from little endian to cpu endian */
+static u32 cramfs_offset_to_cpu(struct cramfs_inode *inode)
+{
+	return inode->offset;
+}
+
+/* Converts a cramfs_inode's namelength field
+   from little endian to cpu endian */
+static u32 cramfs_namelen_to_cpu(struct cramfs_inode *inode)
+{
+	return inode->namelen;
+}
+
+/* Converts a cramfs_inode's size field
+   from little endian to cpu endian */
+static u32 cramfs_size_to_cpu(struct cramfs_inode *inode)
+{
+	return inode->size;
+}
+#else
+#error "Neither __BIG_ENDIAN nor __LITTLE_ENDIAN defined."
+#endif
 
 static int cramfs_iget5_test(struct inode *inode, void *opaque)
 {
@@ -53,13 +110,13 @@ static int cramfs_iget5_test(struct inode *inode, void *opaque)
 
 	/* all empty directories, char, block, pipe, and sock, share inode #1 */
 
-	if ((inode->i_mode != cramfs_inode->mode) ||
+	if ((inode->i_mode != le16_to_cpu(cramfs_inode->mode)) ||
 	    (inode->i_gid != cramfs_inode->gid) ||
-	    (inode->i_uid != cramfs_inode->uid))
+	    (inode->i_uid != le16_to_cpu(cramfs_inode->uid)))
 		return 0; /* does not match */
 
 	if ((S_ISCHR(inode->i_mode) || S_ISBLK(inode->i_mode)) &&
-	    (inode->i_rdev != old_decode_dev(cramfs_inode->size)))
+	    (inode->i_rdev != old_decode_dev(cramfs_size_to_cpu(cramfs_inode))))
 		return 0; /* does not match */
 
 	return 1; /* matches */
@@ -69,10 +126,10 @@ static int cramfs_iget5_set(struct inode *inode, void *opaque)
 {
 	static struct timespec zerotime;
 	struct cramfs_inode *cramfs_inode = opaque;
-	inode->i_mode = cramfs_inode->mode;
-	inode->i_uid = cramfs_inode->uid;
-	inode->i_size = cramfs_inode->size;
-	inode->i_blocks = (cramfs_inode->size - 1) / 512 + 1;
+	inode->i_mode = le16_to_cpu(cramfs_inode->mode);
+	inode->i_uid = le16_to_cpu(cramfs_inode->uid);
+	inode->i_size = cramfs_size_to_cpu(cramfs_inode);
+	inode->i_blocks = (cramfs_size_to_cpu(cramfs_inode) - 1) / 512 + 1;
 	inode->i_gid = cramfs_inode->gid;
 	/* Struct copy intentional */
 	inode->i_mtime = inode->i_atime = inode->i_ctime = zerotime;
@@ -94,7 +151,7 @@ static int cramfs_iget5_set(struct inode *inode, void *opaque)
 		inode->i_size = 0;
 		inode->i_blocks = 0;
 		init_special_inode(inode, inode->i_mode,
-			old_decode_dev(cramfs_inode->size));
+			old_decode_dev(cramfs_size_to_cpu(cramfs_inode)));
 	}
 	return 0;
 }
@@ -256,53 +313,57 @@ static int cramfs_fill_super(struct super_block *sb, void *data, int silent)
 	mutex_unlock(&read_mutex);
 
 	/* Do sanity checks on the superblock */
-	if (super.magic != CRAMFS_MAGIC) {
-		/* check for wrong endianess */
-		if (super.magic == CRAMFS_MAGIC_WEND) {
-			if (!silent)
-				printk(KERN_ERR "cramfs: wrong endianess\n");
-			goto out;
-		}
-
+	if (le32_to_cpu(super.magic) != CRAMFS_MAGIC &&
+	    le32_to_cpu(super.magic) != CRAMFS_MAGIC_WEND) {
 		/* check at 512 byte offset */
 		mutex_lock(&read_mutex);
 		memcpy(&super, cramfs_read(sb, 512, sizeof(super)), sizeof(super));
 		mutex_unlock(&read_mutex);
-		if (super.magic != CRAMFS_MAGIC) {
-			if (super.magic == CRAMFS_MAGIC_WEND && !silent)
-				printk(KERN_ERR "cramfs: wrong endianess\n");
-			else if (!silent)
+
+		if (le32_to_cpu(super.magic) == CRAMFS_MAGIC_WEND)
+			goto other_endian;
+		else if (le32_to_cpu(super.magic) != CRAMFS_MAGIC) {
+			if (!silent)
 				printk(KERN_ERR "cramfs: wrong magic\n");
+
 			goto out;
 		}
 	}
+	/* check for wrong endianess */
+	else if (le32_to_cpu(super.magic) == CRAMFS_MAGIC_WEND) {
+other_endian:
+		if (!silent)
+			printk(KERN_ERR "cramfs: filesystems in big endian format are not supported any longer.\n");
+
+		goto out;
+	}
 
 	/* get feature flags first */
-	if (super.flags & ~CRAMFS_SUPPORTED_FLAGS) {
+	if (le32_to_cpu(super.flags) & ~CRAMFS_SUPPORTED_FLAGS) {
 		printk(KERN_ERR "cramfs: unsupported filesystem features\n");
 		goto out;
 	}
 
 	/* Check that the root inode is in a sane state */
-	if (!S_ISDIR(super.root.mode)) {
+	if (!S_ISDIR(le16_to_cpu(super.root.mode))) {
 		printk(KERN_ERR "cramfs: root is not a directory\n");
 		goto out;
 	}
-	root_offset = super.root.offset << 2;
+	root_offset = cramfs_offset_to_cpu(&super.root) << 2;
 	if (super.flags & CRAMFS_FLAG_FSID_VERSION_2) {
-		sbi->size=super.size;
-		sbi->blocks=super.fsid.blocks;
-		sbi->files=super.fsid.files;
+		sbi->size = le32_to_cpu(super.size);
+		sbi->blocks = le32_to_cpu(super.fsid.blocks);
+		sbi->files = le32_to_cpu(super.fsid.files);
 	} else {
 		sbi->size=1<<28;
 		sbi->blocks=0;
 		sbi->files=0;
 	}
-	sbi->magic=super.magic;
-	sbi->flags=super.flags;
+	sbi->magic = le32_to_cpu(super.magic);
+	sbi->flags = le32_to_cpu(super.flags);
 	if (root_offset == 0)
 		printk(KERN_INFO "cramfs: empty filesystem");
-	else if (!(super.flags & CRAMFS_FLAG_SHIFTED_ROOT_OFFSET) &&
+	else if (!(le32_to_cpu(super.flags) & CRAMFS_FLAG_SHIFTED_ROOT_OFFSET) &&
 		 ((root_offset != sizeof(struct cramfs_super)) &&
 		  (root_offset != 512 + sizeof(struct cramfs_super))))
 	{
@@ -383,10 +444,10 @@ static int cramfs_readdir(struct file *filp, void *dirent, filldir_t filldir)
 		 * and the name padded out to 4-byte boundaries
 		 * with zeroes.
 		 */
-		namelen = de->namelen << 2;
+		namelen = cramfs_namelen_to_cpu(de) << 2;
 		memcpy(buf, name, namelen);
 		ino = CRAMINO(de);
-		mode = de->mode;
+		mode = le16_to_cpu(de->mode);
 		mutex_unlock(&read_mutex);
 		nextoffset = offset + sizeof(*de) + namelen;
 		for (;;) {
@@ -432,7 +493,7 @@ static struct dentry * cramfs_lookup(struct inode *dir, struct dentry *dentry, s
 		if (sorted && (dentry->d_name.name[0] < name[0]))
 			break;
 
-		namelen = de->namelen << 2;
+		namelen = cramfs_namelen_to_cpu(de) << 2;
 		offset += sizeof(*de) + namelen;
 
 		/* Quick check that the name is roughly the right length */
@@ -484,8 +545,8 @@ static int cramfs_readpage(struct file *file, struct page * page)
 		start_offset = OFFSET(inode) + maxblock*4;
 		mutex_lock(&read_mutex);
 		if (page->index)
-			start_offset = *(u32 *) cramfs_read(sb, blkptr_offset-4, 4);
-		compr_len = (*(u32 *) cramfs_read(sb, blkptr_offset, 4) - start_offset);
+			start_offset = le32_to_cpu(*(u32 *) cramfs_read(sb, blkptr_offset-4, 4));
+		compr_len = le32_to_cpu(*(u32 *) cramfs_read(sb, blkptr_offset, 4)) - start_offset;
 		mutex_unlock(&read_mutex);
 		pgdata = kmap(page);
 		if (compr_len == 0)