[PATCH] remove some usesless casts

[PATCH] remove some usesless casts

[Jörn Engel]

Squashfs is extremely cast-happy.  This patch makes it less so.

Jörn

-- 
If you're willing to restrict the flexibility of your approach,
you can almost always do something better.
-- John Carmack


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c |   63 ++++++++++++++++++++++------------------------------
 1 files changed, 27 insertions(+), 36 deletions(-)

--- linux-2.6.12-rc2cow/fs/squashfs/inode.c~squashfs_cu1	2005-04-20 07:52:46.000000000 +0200
+++ linux-2.6.12-rc2cow/fs/squashfs/inode.c	2005-04-20 08:11:10.254367656 +0200
@@ -111,7 +111,7 @@ struct inode_operations squashfs_dir_ino
 static struct buffer_head *get_block_length(struct super_block *s,
 				int *cur_index, int *offset, int *c_byte)
 {
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)s->s_fs_info;
+	squashfs_sb_info *msBlk = s->s_fs_info;
 	unsigned short temp;
 	struct buffer_head *bh;
 
@@ -176,7 +176,7 @@ unsigned int squashfs_read_data(struct s
 			unsigned int index, unsigned int length,
 			unsigned int *next_index)
 {
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)s->s_fs_info;
+	squashfs_sb_info *msBlk = s->s_fs_info;
 	struct buffer_head *bh[((SQUASHFS_FILE_MAX_SIZE - 1) >>
 			msBlk->devblksize_log2) + 2];
 	unsigned int offset = index & ((1 << msBlk->devblksize_log2) - 1);
@@ -285,7 +285,7 @@ int squashfs_get_cached_block(struct sup
 				int length, unsigned int *next_block,
 				unsigned int *next_offset)
 {
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)s->s_fs_info;
+	squashfs_sb_info *msBlk = s->s_fs_info;
 	int n, i, bytes, return_length = length;
 	unsigned int next_index;
 
@@ -390,7 +390,7 @@ static int get_fragment_location(struct 
 				unsigned int *fragment_start_block,
 				unsigned int *fragment_size)
 {
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)s->s_fs_info;
+	squashfs_sb_info *msBlk = s->s_fs_info;
 	unsigned int start_block =
 		msBlk->fragment_index[SQUASHFS_FRAGMENT_INDEX(fragment)];
 	int offset = SQUASHFS_FRAGMENT_INDEX_OFFSET(fragment);
@@ -434,7 +434,7 @@ static struct squashfs_fragment_cache *g
 					int length)
 {
 	int i, n;
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)s->s_fs_info;
+	squashfs_sb_info *msBlk = s->s_fs_info;
 
 	for (;;) {
 		down(&msBlk->fragment_mutex);
@@ -466,8 +466,7 @@ static struct squashfs_fragment_cache *g
 				SQUASHFS_CACHED_FRAGMENTS;
 			
 			if (msBlk->fragment[i].data == NULL)
-				if (!(msBlk->fragment[i].data =
-						(unsigned char *) kmalloc
+				if (!(msBlk->fragment[i].data = kmalloc
 						(SQUASHFS_FILE_MAX_SIZE,
 						 GFP_KERNEL))) {
 					ERROR("Failed to allocate fragment "
@@ -509,7 +508,7 @@ static struct squashfs_fragment_cache *g
 static struct inode *squashfs_new_inode(struct super_block *s,
 		squashfs_base_inode_header *inodeb, unsigned int ino)
 {
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)s->s_fs_info;
+	squashfs_sb_info *msBlk = s->s_fs_info;
 	squashfs_super_block *sBlk = &msBlk->sBlk;
 	struct inode *i = new_inode(s);
 
@@ -535,7 +534,7 @@ static struct inode *squashfs_new_inode(
 static struct inode *squashfs_iget(struct super_block *s, squashfs_inode inode)
 {
 	struct inode *i;
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)s->s_fs_info;
+	squashfs_sb_info *msBlk = s->s_fs_info;
 	squashfs_super_block *sBlk = &msBlk->sBlk;
 	unsigned int block = SQUASHFS_INODE_BLK(inode) +
 		sBlk->inode_table_start;
@@ -837,13 +836,12 @@ static int squashfs_fill_super(struct su
 
 	TRACE("Entered squashfs_read_superblock\n");
 
-	if (!(s->s_fs_info = (void *) kmalloc(sizeof(squashfs_sb_info),
-						GFP_KERNEL))) {
+	if (!(s->s_fs_info = kmalloc(sizeof(squashfs_sb_info), GFP_KERNEL))) {
 		ERROR("Failed to allocate superblock\n");
 		return -ENOMEM;
 	}
 	memset(s->s_fs_info, 0, sizeof(squashfs_sb_info));
-	msBlk = (squashfs_sb_info *) s->s_fs_info;
+	msBlk = s->s_fs_info;
 	sBlk = &msBlk->sBlk;
 	
 	msBlk->devblksize = sb_min_blocksize(s, BLOCK_SIZE);
@@ -914,8 +912,7 @@ static int squashfs_fill_super(struct su
 	s->s_op = &squashfs_ops;
 
 	/* Init inode_table block pointer array */
-	if (!(msBlk->block_cache = (squashfs_cache *)
-					kmalloc(sizeof(squashfs_cache) *
+	if (!(msBlk->block_cache = kmalloc(sizeof(squashfs_cache) *
 					SQUASHFS_CACHED_BLKS, GFP_KERNEL))) {
 		ERROR("Failed to allocate block cache\n");
 		goto failed_mount;
@@ -931,15 +928,14 @@ static int squashfs_fill_super(struct su
 					SQUASHFS_METADATA_SIZE :
 					sBlk->block_size;
 
-	if (!(msBlk->read_data = (char *) kmalloc(msBlk->read_size,
-					GFP_KERNEL))) {
+	if (!(msBlk->read_data = kmalloc(msBlk->read_size, GFP_KERNEL))) {
 		ERROR("Failed to allocate read_data block\n");
 		goto failed_mount;
 	}
 
 	/* Allocate read_page block */
 	if (sBlk->block_size > PAGE_CACHE_SIZE) {
-		if (!(msBlk->read_page = (char *) kmalloc(sBlk->block_size,
+		if (!(msBlk->read_page = kmalloc(sBlk->block_size,
 					GFP_KERNEL))) {
 			ERROR("Failed to allocate read_page block\n");
 			goto failed_mount;
@@ -948,7 +944,7 @@ static int squashfs_fill_super(struct su
 		msBlk->read_page = NULL;
 
 	/* Allocate uid and gid tables */
-	if (!(msBlk->uid = (squashfs_uid *) kmalloc((sBlk->no_uids +
+	if (!(msBlk->uid = kmalloc((sBlk->no_uids +
 					sBlk->no_guids) * sizeof(squashfs_uid),
 					GFP_KERNEL))) {
 		ERROR("Failed to allocate uid/gid table\n");
@@ -982,9 +978,7 @@ static int squashfs_fill_super(struct su
 	if (sBlk->s_major == 1 && squashfs_1_0_supported(msBlk))
 		goto allocate_root;
 
-	if (!(msBlk->fragment = (struct squashfs_fragment_cache *)
-					kmalloc(sizeof(struct
-					squashfs_fragment_cache) *
+	if (!(msBlk->fragment = kmalloc(sizeof(struct squashfs_fragment_cache) *
 					SQUASHFS_CACHED_FRAGMENTS,
 					GFP_KERNEL))) {
 		ERROR("Failed to allocate fragment block cache\n");
@@ -1000,8 +994,7 @@ static int squashfs_fill_super(struct su
 	msBlk->next_fragment = 0;
 
 	/* Allocate fragment index table */
-	if (!(msBlk->fragment_index = (squashfs_fragment_index *)
-					kmalloc(SQUASHFS_FRAGMENT_INDEX_BYTES
+	if (!(msBlk->fragment_index = kmalloc(SQUASHFS_FRAGMENT_INDEX_BYTES
 					(sBlk->fragments), GFP_KERNEL))) {
 		ERROR("Failed to allocate uid/gid table\n");
 		goto failed_mount;
@@ -1127,7 +1120,7 @@ static unsigned int read_blocklist(struc
 				int readahead_blks, char *block_list,
 				unsigned short **block_p, unsigned int *bsize)
 {
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)inode->i_sb->s_fs_info;
+	squashfs_sb_info *msBlk = inode->i_sb->s_fs_info;
 	unsigned int *block_listp;
 	int i = 0;
 	int block_ptr = SQUASHFS_I(inode)->block_list_start;
@@ -1182,7 +1175,7 @@ static unsigned int read_blocklist(struc
 static int squashfs_readpage(struct file *file, struct page *page)
 {
 	struct inode *inode = page->mapping->host;
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)inode->i_sb->s_fs_info;
+	squashfs_sb_info *msBlk = inode->i_sb->s_fs_info;
 	squashfs_super_block *sBlk = &msBlk->sBlk;
 	unsigned char block_list[SIZE];
 	unsigned int bsize, block, i = 0, bytes = 0, byte_offset = 0;
@@ -1297,7 +1290,7 @@ skip_read:
 static int squashfs_readpage4K(struct file *file, struct page *page)
 {
 	struct inode *inode = page->mapping->host;
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)inode->i_sb->s_fs_info;
+	squashfs_sb_info *msBlk = inode->i_sb->s_fs_info;
 	squashfs_super_block *sBlk = &msBlk->sBlk;
 	unsigned char block_list[SIZE];
 	unsigned int bsize, block, bytes = 0;
@@ -1359,7 +1352,7 @@ static int get_dir_index_using_offset(st
 				unsigned int index_offset, int i_count,
 				long long f_pos)
 {
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)s->s_fs_info;
+	squashfs_sb_info *msBlk = s->s_fs_info;
 	squashfs_super_block *sBlk = &msBlk->sBlk;
 	int i, length = 0;
 	squashfs_dir_index index;
@@ -1406,7 +1399,7 @@ static int get_dir_index_using_name(stru
 				unsigned int index_offset, int i_count,
 				const char *name, int size)
 {
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)s->s_fs_info;
+	squashfs_sb_info *msBlk = s->s_fs_info;
 	squashfs_super_block *sBlk = &msBlk->sBlk;
 	int i, length = 0;
 	char buffer[sizeof(squashfs_dir_index) + SQUASHFS_NAME_LEN + 1];
@@ -1453,7 +1446,7 @@ static int get_dir_index_using_name(stru
 static int squashfs_readdir(struct file *file, void *dirent, filldir_t filldir)
 {
 	struct inode *i = file->f_dentry->d_inode;
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)i->i_sb->s_fs_info;
+	squashfs_sb_info *msBlk = i->i_sb->s_fs_info;
 	squashfs_super_block *sBlk = &msBlk->sBlk;
 	int next_block = SQUASHFS_I(i)->start_block +
 		sBlk->directory_table_start,
@@ -1562,7 +1555,7 @@ static struct dentry *squashfs_lookup(st
 	const unsigned char *name = dentry->d_name.name;
 	int len = dentry->d_name.len;
 	struct inode *inode = NULL;
-	squashfs_sb_info *msBlk = (squashfs_sb_info *)i->i_sb->s_fs_info;
+	squashfs_sb_info *msBlk = i->i_sb->s_fs_info;
 	squashfs_super_block *sBlk = &msBlk->sBlk;
 	int next_block = SQUASHFS_I(i)->start_block +
 				sBlk->directory_table_start,
@@ -1666,7 +1659,7 @@ static void squashfs_put_super(struct su
 	int i;
 
 	if (s->s_fs_info) {
-		squashfs_sb_info *sbi = (squashfs_sb_info *) s->s_fs_info;
+		squashfs_sb_info *sbi = s->s_fs_info;
 		if (sbi->block_cache)
 			for (i = 0; i < SQUASHFS_CACHED_BLKS; i++)
 				if (sbi->block_cache[i].block !=
@@ -1703,8 +1696,7 @@ static int __init init_squashfs_fs(void)
 	printk(KERN_INFO "squashfs: version 2.1 (2005/03/14) "
 		"Phillip Lougher\n");
 
-	if (!(stream.workspace = (char *)
-				vmalloc(zlib_inflate_workspacesize()))) {
+	if (!(stream.workspace = vmalloc(zlib_inflate_workspacesize()))) {
 		ERROR("Failed to allocate zlib workspace\n");
 		destroy_inodecache();
 		return -ENOMEM;
@@ -1733,8 +1725,7 @@ static kmem_cache_t * squashfs_inode_cac
 static struct inode *squashfs_alloc_inode(struct super_block *sb)
 {
 	struct squashfs_inode_info *ei;
-	ei = (struct squashfs_inode_info *)
-		kmem_cache_alloc(squashfs_inode_cachep, SLAB_KERNEL);
+	ei = kmem_cache_alloc(squashfs_inode_cachep, SLAB_KERNEL);
 	if (!ei)
 		return NULL;
 	return &ei->vfs_inode;
@@ -1749,7 +1740,7 @@ static void squashfs_destroy_inode(struc
 
 static void init_once(void * foo, kmem_cache_t * cachep, unsigned long flags)
 {
-	struct squashfs_inode_info *ei = (struct squashfs_inode_info *) foo;
+	struct squashfs_inode_info *ei = foo;
 
 	if ((flags & (SLAB_CTOR_VERIFY|SLAB_CTOR_CONSTRUCTOR)) ==
 							SLAB_CTOR_CONSTRUCTOR)

Re: [PATCH] remove some usesless casts

[Phillip Lougher]

Jörn Engel wrote:
> Squashfs is extremely cast-happy.  This patch makes it less so.
> 
> Jörn
> 

Hi,

Thanks for the patch.  Unnecessary casts were one of the things 
mentioned when I submitted the patches to the LKML, and therefore I 
suspect most of them have been already fixed (but I will apply your 
patch to check).

I will send revised patches to the LKML soon, most of the issues raised 
by the comments have been fixed, the current delay is being caused by 
the 4GB limit re-work.

Regards

Phillip

Re: [PATCH] remove some usesless casts

[Phillip Lougher]

Jörn Engel wrote:

> Your definition of _unnecessary_ casts may differ from mine.
> Basically, every cast is unnecessary, except for maybe one or two - if
> that many.
> 

Well we agree to differ then.  In my experience casts are sometimes 
necessary, and are often less clumsy than the alternatives (such as 
unions).  This is probably a generational thing, the fashion today is to 
make languages much more strongly typechecked than before.

Re: [PATCH] remove some usesless casts

[Jörn Engel]

On Wed, 20 April 2005 16:20:10 +0100, Phillip Lougher wrote:
> Jörn Engel wrote:
> >Squashfs is extremely cast-happy.  This patch makes it less so.
> 
> Thanks for the patch.  Unnecessary casts were one of the things 
> mentioned when I submitted the patches to the LKML, and therefore I 
> suspect most of them have been already fixed (but I will apply your 
> patch to check).

Your definition of _unnecessary_ casts may differ from mine.
Basically, every cast is unnecessary, except for maybe one or two - if
that many.

> I will send revised patches to the LKML soon, most of the issues raised 
> by the comments have been fixed, the current delay is being caused by 
> the 4GB limit re-work.

There are three more patches in my queue, which I'll send after first
coffee or so.  After those I'll ignore squashfs for a while (until you
send current code or so).

Jörn

-- 
The story so far:
In the beginning the Universe was created.  This has made a lot
of people very angry and been widely regarded as a bad move.
-- Douglas Adams

Re: [PATCH] remove some usesless casts

[Jörn Engel]

On Wed, 20 April 2005 21:51:15 +0100, Phillip Lougher wrote:
> Jörn Engel wrote:
> 
> >Your definition of _unnecessary_ casts may differ from mine.
> >Basically, every cast is unnecessary, except for maybe one or two - if
> >that many.
> 
> Well we agree to differ then.  In my experience casts are sometimes 
> necessary, and are often less clumsy than the alternatives (such as 
> unions).  This is probably a generational thing, the fashion today is to 
> make languages much more strongly typechecked than before.

I never claimed to replace the casts with unions. ;)

Jörn

-- 
Homo Sapiens is a goal, not a description.
-- unknown

[PATCH 2/4] squashfs: kmalloc (less stack, less casts)

[Jörn Engel]

Jörn

-- 
Rules of Optimization:
Rule 1: Don't do it.
Rule 2 (for experts only): Don't do it yet.
-- M.A. Jackson 


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

--- linux-2.6.12-rc2cow/fs/squashfs/inode.c~squashfs_cu2	2005-04-20 08:11:10.000000000 +0200
+++ linux-2.6.12-rc2cow/fs/squashfs/inode.c	2005-04-20 16:34:43.619956672 +0200
@@ -1453,11 +1453,14 @@ static int squashfs_readdir(struct file 
 		next_offset = SQUASHFS_I(i)->offset, length = 0, dirs_read = 0,
 		dir_count;
 	squashfs_dir_header dirh;
-	char buffer[sizeof(squashfs_dir_entry) + SQUASHFS_NAME_LEN + 1];
-	squashfs_dir_entry *dire = (squashfs_dir_entry *) buffer;
+	squashfs_dir_entry *dire;
 
 	TRACE("Entered squashfs_readdir [%x:%x]\n", next_block, next_offset);
 
+	dire = kmalloc(sizeof(*dire) + SQUASHFS_NAME_LEN + 1, GFP_KERNEL);
+	if (!dire)
+		return -ENOMEM;
+
 	length = get_dir_index_using_offset(i->i_sb, &next_block, &next_offset,
 				SQUASHFS_I(i)->u.s2.directory_index_start,
 				SQUASHFS_I(i)->u.s2.directory_index_offset,
@@ -1533,6 +1536,7 @@ static int squashfs_readdir(struct file 
 					squashfs_filetype_table[dire->type])
 					< 0) {
 				TRACE("Filldir returned less than 0\n");
+				kfree(dire);
 				return dirs_read;
 			}
 			file->f_pos = length;
@@ -1540,11 +1544,13 @@ static int squashfs_readdir(struct file 
 		}
 	}
 
+	kfree(dire);
 	return dirs_read;
 
 failed_read:
 	ERROR("Unable to read directory block [%x:%x]\n", next_block,
 		next_offset);
+	kfree(dire);
 	return 0;
 }
 
@@ -1562,12 +1568,15 @@ static struct dentry *squashfs_lookup(st
 				next_offset = SQUASHFS_I(i)->offset, length = 0,
 				dir_count;
 	squashfs_dir_header dirh;
-	char buffer[sizeof(squashfs_dir_entry) + SQUASHFS_NAME_LEN];
-	squashfs_dir_entry *dire = (squashfs_dir_entry *) buffer;
+	squashfs_dir_entry *dire;
 	int sorted = sBlk->s_major == 2 && sBlk->s_minor >= 1;
 
 	TRACE("Entered squashfs_lookup [%x:%x]\n", next_block, next_offset);
 
+	dire = kmalloc(sizeof(*dire) + SQUASHFS_NAME_LEN + 1, GFP_KERNEL);
+	if (!dire)
+		return ERR_PTR(-ENOMEM);
+
 	length = get_dir_index_using_name(i->i_sb, &next_block, &next_offset,
 				SQUASHFS_I(i)->u.s2.directory_index_start,
 				SQUASHFS_I(i)->u.s2.directory_index_offset,
@@ -1645,7 +1654,8 @@ static struct dentry *squashfs_lookup(st
 
 exit_loop:
 	d_add(dentry, inode);
-	return ERR_PTR(0);
+	kfree(dire);
+	return NULL;
 
 failed_read:
 	ERROR("Unable to read directory block [%x:%x]\n", next_block,

[PATCH 3/4] squashfs: (void*)ify squashfs_get_cached_block

[Jörn Engel]

Jörn

-- 
It's just what we asked for, but not what we want!
-- anonymous


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c    |   58 ++++++++++++++++++++++++-------------------------
 fs/squashfs/squashfs.h |    2 -
 2 files changed, 30 insertions(+), 30 deletions(-)

--- linux-2.6.12-rc2cow/fs/squashfs/inode.c~squashfs_cu3	2005-04-20 16:34:43.619956672 +0200
+++ linux-2.6.12-rc2cow/fs/squashfs/inode.c	2005-04-20 16:40:47.498638704 +0200
@@ -280,7 +280,7 @@ read_failure:
 }
 
 
-int squashfs_get_cached_block(struct super_block *s, char *buffer,
+int squashfs_get_cached_block(struct super_block *s, void *buffer,
 				unsigned int block, unsigned int offset,
 				int length, unsigned int *next_block,
 				unsigned int *next_offset)
@@ -399,14 +399,14 @@ static int get_fragment_location(struct 
 	if (msBlk->swap) {
 		squashfs_fragment_entry sfragment_entry;
 
-		if (!squashfs_get_cached_block(s, (char *) &sfragment_entry,
+		if (!squashfs_get_cached_block(s, &sfragment_entry,
 					start_block, offset,
 					sizeof(sfragment_entry), &start_block,
 					&offset))
 			return 0;
 		SQUASHFS_SWAP_FRAGMENT_ENTRY(&fragment_entry, &sfragment_entry);
 	} else
-		if (!squashfs_get_cached_block(s, (char *) &fragment_entry,
+		if (!squashfs_get_cached_block(s, &fragment_entry,
 					start_block, offset,
 					sizeof(fragment_entry), &start_block,
 					&offset))
@@ -549,14 +549,14 @@ static struct inode *squashfs_iget(struc
 	if (msBlk->swap) {
 		squashfs_base_inode_header sinodeb;
 
-		if (!squashfs_get_cached_block(s, (char *) &sinodeb, block,
+		if (!squashfs_get_cached_block(s, &sinodeb, block,
 					offset, sizeof(sinodeb), &next_block,
 					&next_offset))
 			goto failed_read;
 		SQUASHFS_SWAP_BASE_INODE_HEADER(&inodeb, &sinodeb,
 					sizeof(sinodeb));
 	} else
-		if (!squashfs_get_cached_block(s, (char *) &inodeb, block,
+		if (!squashfs_get_cached_block(s, &inodeb, block,
 					offset, sizeof(inodeb), &next_block,
 					&next_offset))
 			goto failed_read;
@@ -569,7 +569,7 @@ static struct inode *squashfs_iget(struc
 			if (msBlk->swap) {
 				squashfs_reg_inode_header sinodep;
 
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&sinodep, block, offset,
 						sizeof(sinodep), &next_block,
 						&next_offset))
@@ -577,7 +577,7 @@ static struct inode *squashfs_iget(struc
 				SQUASHFS_SWAP_REG_INODE_HEADER(&inodep,
 								&sinodep);
 			} else
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&inodep, block, offset,
 						sizeof(inodep), &next_block,
 						&next_offset))
@@ -624,7 +624,7 @@ static struct inode *squashfs_iget(struc
 			if (msBlk->swap) {
 				squashfs_dir_inode_header sinodep;
 
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&sinodep, block, offset,
 						sizeof(sinodep), &next_block,
 						&next_offset))
@@ -632,7 +632,7 @@ static struct inode *squashfs_iget(struc
 				SQUASHFS_SWAP_DIR_INODE_HEADER(&inodep,
 								&sinodep);
 			} else
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&inodep, block, offset,
 						sizeof(inodep), &next_block,
 						&next_offset))
@@ -664,7 +664,7 @@ static struct inode *squashfs_iget(struc
 			if (msBlk->swap) {
 				squashfs_ldir_inode_header sinodep;
 
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&sinodep, block, offset,
 						sizeof(sinodep), &next_block,
 						&next_offset))
@@ -672,7 +672,7 @@ static struct inode *squashfs_iget(struc
 				SQUASHFS_SWAP_LDIR_INODE_HEADER(&inodep,
 								&sinodep);
 			} else
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&inodep, block, offset,
 						sizeof(inodep), &next_block,
 						&next_offset))
@@ -708,7 +708,7 @@ static struct inode *squashfs_iget(struc
 			if (msBlk->swap) {
 				squashfs_symlink_inode_header sinodep;
 
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&sinodep, block, offset,
 						sizeof(sinodep), &next_block,
 						&next_offset))
@@ -716,7 +716,7 @@ static struct inode *squashfs_iget(struc
 				SQUASHFS_SWAP_SYMLINK_INODE_HEADER(&inodep,
 								&sinodep);
 			} else
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&inodep, block, offset,
 						sizeof(inodep), &next_block,
 						&next_offset))
@@ -745,7 +745,7 @@ static struct inode *squashfs_iget(struc
 			if (msBlk->swap) {
 				squashfs_dev_inode_header sinodep;
 
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&sinodep, block, offset,
 						sizeof(sinodep), &next_block,
 						&next_offset))
@@ -753,7 +753,7 @@ static struct inode *squashfs_iget(struc
 				SQUASHFS_SWAP_DEV_INODE_HEADER(&inodep,
 					&sinodep);
 			} else	
-				if (!squashfs_get_cached_block(s, (char *)
+				if (!squashfs_get_cached_block(s,
 						&inodep, block, offset,
 						sizeof(inodep), &next_block,
 						&next_offset))
@@ -1139,7 +1139,7 @@ static unsigned int read_blocklist(struc
 		if (msBlk->swap) {
 			unsigned char sblock_list[SIZE];
 
-			if (!squashfs_get_cached_block(inode->i_sb, (char *)
+			if (!squashfs_get_cached_block(inode->i_sb,
 					sblock_list, block_ptr,
 					offset, blocks << 2, &block_ptr,
 					&offset)) {
@@ -1150,7 +1150,7 @@ static unsigned int read_blocklist(struc
 			SQUASHFS_SWAP_INTS(((unsigned int *)block_list),
 					((unsigned int *)sblock_list), blocks);
 		} else
-			if (!squashfs_get_cached_block(inode->i_sb, (char *)
+			if (!squashfs_get_cached_block(inode->i_sb,
 					block_list, block_ptr, offset,
 					blocks << 2, &block_ptr,
 					&offset)) {
@@ -1366,13 +1366,13 @@ static int get_dir_index_using_offset(st
 	for (i = 0; i < i_count; i++) {
 		if (msBlk->swap) {
 			squashfs_dir_index sindex;
-			squashfs_get_cached_block(s, (char *) &sindex,
+			squashfs_get_cached_block(s, &sindex,
 					index_start, index_offset,
 					sizeof(sindex), &index_start,
 					&index_offset);
 			SQUASHFS_SWAP_DIR_INDEX(&index, &sindex);
 		} else
-			squashfs_get_cached_block(s, (char *) &index,
+			squashfs_get_cached_block(s, &index,
 					index_start, index_offset,
 					sizeof(index), &index_start,
 					&index_offset);
@@ -1414,13 +1414,13 @@ static int get_dir_index_using_name(stru
 	for (i = 0; i < i_count; i++) {
 		if (msBlk->swap) {
 			squashfs_dir_index sindex;
-			squashfs_get_cached_block(s, (char *) &sindex,
+			squashfs_get_cached_block(s, &sindex,
 					index_start, index_offset,
 					sizeof(sindex), &index_start,
 					&index_offset);
 			SQUASHFS_SWAP_DIR_INDEX(index, &sindex);
 		} else
-			squashfs_get_cached_block(s, (char *) index,
+			squashfs_get_cached_block(s, index,
 					index_start, index_offset,
 					sizeof(squashfs_dir_index),
 					&index_start, &index_offset);
@@ -1472,7 +1472,7 @@ static int squashfs_readdir(struct file 
 		if (msBlk->swap) {
 			squashfs_dir_header sdirh;
 			
-			if (!squashfs_get_cached_block(i->i_sb, (char *) &sdirh,
+			if (!squashfs_get_cached_block(i->i_sb, &sdirh,
 					next_block, next_offset, sizeof(sdirh),
 					&next_block, &next_offset))
 				goto failed_read;
@@ -1480,7 +1480,7 @@ static int squashfs_readdir(struct file 
 			length += sizeof(sdirh);
 			SQUASHFS_SWAP_DIR_HEADER(&dirh, &sdirh);
 		} else {
-			if (!squashfs_get_cached_block(i->i_sb, (char *) &dirh,
+			if (!squashfs_get_cached_block(i->i_sb, &dirh,
 					next_block, next_offset, sizeof(dirh),
 					&next_block, &next_offset))
 				goto failed_read;
@@ -1492,7 +1492,7 @@ static int squashfs_readdir(struct file 
 		while (dir_count--) {
 			if (msBlk->swap) {
 				squashfs_dir_entry sdire;
-				if (!squashfs_get_cached_block(i->i_sb, (char *)
+				if (!squashfs_get_cached_block(i->i_sb,
 						&sdire, next_block, next_offset,
 						sizeof(sdire), &next_block,
 						&next_offset))
@@ -1501,7 +1501,7 @@ static int squashfs_readdir(struct file 
 				length += sizeof(sdire);
 				SQUASHFS_SWAP_DIR_ENTRY(dire, &sdire);
 			} else {
-				if (!squashfs_get_cached_block(i->i_sb, (char *)
+				if (!squashfs_get_cached_block(i->i_sb,
 						dire, next_block, next_offset,
 						sizeof(*dire), &next_block,
 						&next_offset))
@@ -1587,7 +1587,7 @@ static struct dentry *squashfs_lookup(st
 		/* read directory header */
 		if (msBlk->swap) {
 			squashfs_dir_header sdirh;
-			if (!squashfs_get_cached_block(i->i_sb, (char *) &sdirh,
+			if (!squashfs_get_cached_block(i->i_sb, &sdirh,
 					next_block, next_offset, sizeof(sdirh),
 					&next_block, &next_offset))
 				goto failed_read;
@@ -1595,7 +1595,7 @@ static struct dentry *squashfs_lookup(st
 			length += sizeof(sdirh);
 			SQUASHFS_SWAP_DIR_HEADER(&dirh, &sdirh);
 		} else {
-			if (!squashfs_get_cached_block(i->i_sb, (char *) &dirh,
+			if (!squashfs_get_cached_block(i->i_sb, &dirh,
 					next_block, next_offset, sizeof(dirh),
 					&next_block, &next_offset))
 				goto failed_read;
@@ -1607,7 +1607,7 @@ static struct dentry *squashfs_lookup(st
 		while (dir_count--) {
 			if (msBlk->swap) {
 				squashfs_dir_entry sdire;
-				if (!squashfs_get_cached_block(i->i_sb, (char *)
+				if (!squashfs_get_cached_block(i->i_sb,
 						&sdire, next_block,next_offset,
 						sizeof(sdire), &next_block,
 						&next_offset))
@@ -1616,7 +1616,7 @@ static struct dentry *squashfs_lookup(st
 				length += sizeof(sdire);
 				SQUASHFS_SWAP_DIR_ENTRY(dire, &sdire);
 			} else {
-				if (!squashfs_get_cached_block(i->i_sb, (char *)
+				if (!squashfs_get_cached_block(i->i_sb,
 						dire, next_block,next_offset,
 						sizeof(*dire), &next_block,
 						&next_offset))
--- linux-2.6.12-rc2cow/fs/squashfs/squashfs.h~squashfs_cu3	2005-04-20 07:52:50.000000000 +0200
+++ linux-2.6.12-rc2cow/fs/squashfs/squashfs.h	2005-04-20 16:37:15.900806464 +0200
@@ -40,7 +40,7 @@
 extern unsigned int squashfs_read_data(struct super_block *s, char *buffer,
 				unsigned int index, unsigned int length,
 				unsigned int *next_index);
-extern int squashfs_get_cached_block(struct super_block *s, char *buffer,
+extern int squashfs_get_cached_block(struct super_block *s, void *buffer,
 				unsigned int block, unsigned int offset,
 				int length, unsigned int *next_block,
 				unsigned int *next_offset);

[PATCH 4/4] squashfs: (void*)ify squashfs_read_data

[Jörn Engel]

Jörn

-- 
Anything that can go wrong, will.
-- Finagle's Law


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c    |   11 +++++------
 fs/squashfs/squashfs.h |    2 +-
 2 files changed, 6 insertions(+), 7 deletions(-)

--- linux-2.6.12-rc2cow/fs/squashfs/inode.c~squashfs_cu4	2005-04-20 16:40:47.498638704 +0200
+++ linux-2.6.12-rc2cow/fs/squashfs/inode.c	2005-04-20 22:44:04.723347592 +0200
@@ -172,7 +172,7 @@ static struct buffer_head *get_block_len
 }
 
 
-unsigned int squashfs_read_data(struct super_block *s, char *buffer,
+unsigned int squashfs_read_data(struct super_block *s, void *buffer,
 			unsigned int index, unsigned int length,
 			unsigned int *next_index)
 {
@@ -324,7 +324,6 @@ int squashfs_get_cached_block(struct sup
 			if (msBlk->block_cache[i].block ==
 							SQUASHFS_INVALID_BLK) {
 				if (!(msBlk->block_cache[i].data =
-						(unsigned char *)
 						kmalloc(SQUASHFS_METADATA_SIZE,
 						GFP_KERNEL))) {
 					ERROR("Failed to allocate cache"
@@ -854,7 +853,7 @@ static int squashfs_fill_super(struct su
 	init_waitqueue_head(&msBlk->waitq);
 	init_waitqueue_head(&msBlk->fragment_wait_queue);
 
-	if (!squashfs_read_data(s, (char *) sBlk, SQUASHFS_START,
+	if (!squashfs_read_data(s, sBlk, SQUASHFS_START,
 					sizeof(squashfs_super_block) |
 					SQUASHFS_COMPRESSED_BIT_BLOCK, NULL)) {
 		SERROR("unable to read superblock\n");
@@ -955,7 +954,7 @@ static int squashfs_fill_super(struct su
 	if (msBlk->swap) {
 		squashfs_uid suid[sBlk->no_uids + sBlk->no_guids];
 
-		if (!squashfs_read_data(s, (char *) &suid, sBlk->uid_start,
+		if (!squashfs_read_data(s, &suid, sBlk->uid_start,
 					((sBlk->no_uids + sBlk->no_guids) *
 					 sizeof(squashfs_uid)) |
 					SQUASHFS_COMPRESSED_BIT_BLOCK, NULL)) {
@@ -966,7 +965,7 @@ static int squashfs_fill_super(struct su
 		SQUASHFS_SWAP_DATA(msBlk->uid, suid, (sBlk->no_uids +
 			sBlk->no_guids), (sizeof(squashfs_uid) * 8));
 	} else
-		if (!squashfs_read_data(s, (char *) msBlk->uid, sBlk->uid_start,
+		if (!squashfs_read_data(s, msBlk->uid, sBlk->uid_start,
 					((sBlk->no_uids + sBlk->no_guids) *
 					 sizeof(squashfs_uid)) |
 					SQUASHFS_COMPRESSED_BIT_BLOCK, NULL)) {
@@ -1001,7 +1000,7 @@ static int squashfs_fill_super(struct su
 	}
    
 	if (SQUASHFS_FRAGMENT_INDEX_BYTES(sBlk->fragments) &&
-					!squashfs_read_data(s, (char *)
+					!squashfs_read_data(s,
 					msBlk->fragment_index,
 					sBlk->fragment_table_start,
 					SQUASHFS_FRAGMENT_INDEX_BYTES
--- linux-2.6.12-rc2cow/fs/squashfs/squashfs.h~squashfs_cu4	2005-04-20 16:37:15.900806464 +0200
+++ linux-2.6.12-rc2cow/fs/squashfs/squashfs.h	2005-04-20 22:42:45.917327928 +0200
@@ -37,7 +37,7 @@
 
 #define WARNING(s, args...)	printk(KERN_WARNING "SQUASHFS: "s, ## args)
 
-extern unsigned int squashfs_read_data(struct super_block *s, char *buffer,
+extern unsigned int squashfs_read_data(struct super_block *s, void *buffer,
 				unsigned int index, unsigned int length,
 				unsigned int *next_index);
 extern int squashfs_get_cached_block(struct super_block *s, void *buffer,

Re: [PATCH] remove some usesless casts

[Pekka Enberg]

Phillip,

Jörn Engel wrote:
> > Your definition of _unnecessary_ casts may differ from mine.
> > Basically, every cast is unnecessary, except for maybe one or two - if
> > that many.

On 4/20/05, Phillip Lougher <phillip@lougher.demon.co.uk> wrote:
> Well we agree to differ then.  In my experience casts are sometimes
> necessary, and are often less clumsy than the alternatives (such as
> unions).  This is probably a generational thing, the fashion today is to
> make languages much more strongly typechecked than before.

I think Jörn means that if you need an opaque data type, use void
pointers (which are automatically cast to the proper type) and that
all other casts are a design smell (except for the one or two special
cases where you actually need them).

                                 Pekka

Re: [PATCH] remove some usesless casts

[Jörn Engel]

On Thu, 21 April 2005 09:36:18 +0300, Pekka Enberg wrote:
> 
> I think Jörn means that if you need an opaque data type, use void
> pointers (which are automatically cast to the proper type) and that
> all other casts are a design smell (except for the one or two special
> cases where you actually need them).

Two of my patches agree with you, two don't.  Really, in almost all
cases, casts are a Bad Idea(tm).  Almost always, there is _something_
better.  In some cases, this comes down to void pointers, yes.

Jörn

-- 
I can say that I spend most of my time fixing bugs even if I have lots
of new features to implement in mind, but I give bugs more priority.
-- Andrea Arcangeli, 2000

[PATCH 5/10] squashfs: (void*)ify read_blocklist

[Jörn Engel]

Jörn

-- 
With a PC, I always felt limited by the software available. On Unix, 
I am limited only by my knowledge.
-- Peter J. Schoenster


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c            |   16 ++++++++++------
 include/linux/squashfs_fs_sb.h |    6 +++---
 2 files changed, 13 insertions(+), 9 deletions(-)

--- linux-2.6.12-rc3cow/fs/squashfs/inode.c~squashfs_cu5	2005-04-22 07:00:14.806604672 +0200
+++ linux-2.6.12-rc3cow/fs/squashfs/inode.c	2005-04-22 09:17:47.387021752 +0200
@@ -58,7 +58,7 @@ static struct dentry *squashfs_lookup(st
 				struct nameidata *);
 static struct inode *squashfs_iget(struct super_block *s, squashfs_inode inode);
 static unsigned int read_blocklist(struct inode *inode, int index,
-				int readahead_blks, char *block_list,
+				int readahead_blks, void *block_list,
 				unsigned short **block_p, unsigned int *bsize);
 static struct super_block *squashfs_get_sb(struct file_system_type *, int,
 				const char *, void *);
@@ -1116,7 +1116,7 @@ skip_read:
 #define SIZE 256
 
 static unsigned int read_blocklist(struct inode *inode, int index,
-				int readahead_blks, char *block_list,
+				int readahead_blks, void *_block_list,
 				unsigned short **block_p, unsigned int *bsize)
 {
 	squashfs_sb_info *msBlk = inode->i_sb->s_fs_info;
@@ -1125,6 +1125,7 @@ static unsigned int read_blocklist(struc
 	int block_ptr = SQUASHFS_I(inode)->block_list_start;
 	int offset = SQUASHFS_I(inode)->offset;
 	unsigned int block = SQUASHFS_I(inode)->start_block;
+	unsigned int *block_list = _block_list;
 
 	for (;;) {
 		int blocks = (index + readahead_blks - i);
@@ -1136,7 +1137,9 @@ static unsigned int read_blocklist(struc
 		}
 
 		if (msBlk->swap) {
-			unsigned char sblock_list[SIZE];
+			unsigned int *sblock_list = kmalloc(SIZE, GFP_KERNEL);
+			if (!sblock_list)
+				return 0;
 
 			if (!squashfs_get_cached_block(inode->i_sb,
 					sblock_list, block_ptr,
@@ -1144,10 +1147,11 @@ static unsigned int read_blocklist(struc
 					&offset)) {
 				ERROR("Unable to read block list [%d:%x]\n",
 					block_ptr, offset);
+				kfree(sblock_list);
 				return 0;
 			}
-			SQUASHFS_SWAP_INTS(((unsigned int *)block_list),
-					((unsigned int *)sblock_list), blocks);
+			SQUASHFS_SWAP_INTS(block_list, sblock_list, blocks);
+			kfree(sblock_list);
 		} else
 			if (!squashfs_get_cached_block(inode->i_sb,
 					block_list, block_ptr, offset,
@@ -1158,7 +1162,7 @@ static unsigned int read_blocklist(struc
 				return 0;
 			}
 
-		for (block_listp = (unsigned int *) block_list; i < index &&
+		for (block_listp = block_list; i < index &&
 					blocks; i ++, block_listp ++, blocks --)
 			block += SQUASHFS_COMPRESSED_SIZE_BLOCK(*block_listp);
 
--- linux-2.6.12-rc3cow/include/linux/squashfs_fs_sb.h~squashfs_cu5	2005-04-22 07:00:14.693621848 +0200
+++ linux-2.6.12-rc3cow/include/linux/squashfs_fs_sb.h	2005-04-22 09:17:43.833561960 +0200
@@ -59,10 +59,10 @@ typedef struct squashfs_sb_info {
 	struct semaphore	fragment_mutex;
 	wait_queue_head_t	waitq;
 	wait_queue_head_t	fragment_wait_queue;
-	struct inode		*(*iget)(struct super_block *s, squashfs_inode \
+	struct inode		*(*iget)(struct super_block *s, squashfs_inode
 				inode);
-	unsigned int		(*read_blocklist)(struct inode *inode, int \
-				index, int readahead_blks, char *block_list, \
+	unsigned int		(*read_blocklist)(struct inode *inode, int
+				index, int readahead_blks, void *block_list,
 				unsigned short **block_p, unsigned int *bsize);
 	} squashfs_sb_info;
 #endif

[PATCH 6/10] squashfs: conserve stack, remove casts

[Jörn Engel]

Jörn

-- 
All art is but imitation of nature.
-- Lucius Annaeus Seneca


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

--- linux-2.6.12-rc3cow/fs/squashfs/inode.c~squashfs_cu6	2005-04-22 07:21:11.523554656 +0200
+++ linux-2.6.12-rc3cow/fs/squashfs/inode.c	2005-04-22 09:17:45.607292312 +0200
@@ -1405,12 +1405,15 @@ static int get_dir_index_using_name(stru
 	squashfs_sb_info *msBlk = s->s_fs_info;
 	squashfs_super_block *sBlk = &msBlk->sBlk;
 	int i, length = 0;
-	char buffer[sizeof(squashfs_dir_index) + SQUASHFS_NAME_LEN + 1];
-	squashfs_dir_index *index = (squashfs_dir_index *) buffer;
+	squashfs_dir_index *index;
 	char str[SQUASHFS_NAME_LEN + 1];
 
 	TRACE("Entered get_dir_index_using_name, i_count %d\n", i_count);
 
+	index = kmalloc(sizeof(*index) + SQUASHFS_NAME_LEN + 1, GFP_KERNEL);
+	if (!index)
+		return -ENOMEM;
+
 	strncpy(str, name, size);
 	str[size] = '\0';
 
@@ -1442,6 +1445,7 @@ static int get_dir_index_using_name(stru
 	}
 
 	*next_offset = (length + *next_offset) % SQUASHFS_METADATA_SIZE;
+	kfree(index);
 	return length;
 }
 

[PATCH 6/10] squashfs: remove one cast

[Jörn Engel]

Jörn

-- 
/* Keep these two variables together */
int bar;


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

--- linux-2.6.12-rc3cow/fs/squashfs/inode.c~squashfs_cu7	2005-04-22 07:25:24.146150184 +0200
+++ linux-2.6.12-rc3cow/fs/squashfs/inode.c	2005-04-22 09:17:43.831562264 +0200
@@ -1050,7 +1050,8 @@ failed_mount:
 
 static int squashfs_statfs(struct super_block *s, struct kstatfs *buf)
 {
-	squashfs_super_block *sBlk = &((squashfs_sb_info *)s->s_fs_info)->sBlk;
+	squashfs_sb_info *sb = s->s_fs_info;
+	squashfs_super_block *sBlk = &sb->sBlk;
 
 	TRACE("Entered squashfs_statfs\n");
 

[PATCH 8/10] squashfs: add swabbing infrastructure

[Jörn Engel]

Jörn

-- 
Correctness comes second.
Features come third.
Performance comes last.
Maintainability is needed for all of them.


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c            |   16 ++++++++++++++++
 include/linux/squashfs_fs_sb.h |    3 +++
 2 files changed, 19 insertions(+)

--- linux-2.6.12-rc3cow/fs/squashfs/inode.c~squashfs_cu8	2005-04-22 08:09:25.993528336 +0200
+++ linux-2.6.12-rc3cow/fs/squashfs/inode.c	2005-04-22 09:17:42.136819904 +0200
@@ -43,6 +43,16 @@
 #include <linux/vmalloc.h>
 #include "squashfs.h"
 
+
+#define SQUASHFS_SWAB(XX)					\
+u##XX squashfs_swab##XX(u##XX x)	{ return swab##XX(x); }	\
+u##XX squashfs_ident##XX(u##XX x)	{ return x; }
+
+SQUASHFS_SWAB(16)
+SQUASHFS_SWAB(32)
+SQUASHFS_SWAB(64)
+
+
 static void squashfs_put_super(struct super_block *);
 static int squashfs_statfs(struct super_block *, struct kstatfs *);
 static int squashfs_symlink_readpage(struct file *file, struct page *page);
@@ -862,6 +872,9 @@ static int squashfs_fill_super(struct su
 
 	/* Check it is a SQUASHFS superblock */
 	msBlk->swap = 0;
+	msBlk->swab16 = squashfs_ident16;
+	msBlk->swab32 = squashfs_ident32;
+	msBlk->swab64 = squashfs_ident64;
 	if ((s->s_magic = sBlk->s_magic) != SQUASHFS_MAGIC) {
 		if (sBlk->s_magic == SQUASHFS_MAGIC_SWAP) {
 			squashfs_super_block sblk;
@@ -872,6 +885,9 @@ static int squashfs_fill_super(struct su
 			SQUASHFS_SWAP_SUPER_BLOCK(&sblk, sBlk);
 			memcpy(sBlk, &sblk, sizeof(squashfs_super_block));
 			msBlk->swap = 1;
+			msBlk->swab16 = squashfs_swab16;
+			msBlk->swab32 = squashfs_swab32;
+			msBlk->swab64 = squashfs_swab64;
 		} else  {
 			SERROR("Can't find a SQUASHFS superblock on %s\n",
 							bdevname(s->s_bdev, b));
--- linux-2.6.12-rc3cow/include/linux/squashfs_fs_sb.h~squashfs_cu8	2005-04-22 07:13:33.526180840 +0200
+++ linux-2.6.12-rc3cow/include/linux/squashfs_fs_sb.h	2005-04-22 08:54:52.891976760 +0200
@@ -44,6 +44,9 @@ typedef struct squashfs_sb_info {
 	int			devblksize;
 	int			devblksize_log2;
 	int			swap;
+	u16			(*swab16)(u16);
+	u32			(*swab32)(u32);
+	u64			(*swab64)(u64);
 	squashfs_cache		*block_cache;
 	struct squashfs_fragment_cache	*fragment;
 	int			next_cache;

[PATCH 9/10] squashfs: remove some more casts

[Jörn Engel]

Jörn

-- 
Ninety percent of everything is crap.
-- Sturgeon's Law


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

--- linux-2.6.12-rc3cow/fs/squashfs/inode.c~squashfs_cu9	2005-04-22 08:53:27.741921536 +0200
+++ linux-2.6.12-rc3cow/fs/squashfs/inode.c	2005-04-22 09:17:39.795175888 +0200
@@ -124,39 +124,35 @@ static struct buffer_head *get_block_len
 	squashfs_sb_info *msBlk = s->s_fs_info;
 	unsigned short temp;
 	struct buffer_head *bh;
+	unsigned char *data;
 
 	if (!(bh = sb_bread(s, *cur_index)))
 		return NULL;
 
 	if (msBlk->devblksize - *offset == 1) {
+		data = bh->b_data;
 		if (msBlk->swap)
-			((unsigned char *) &temp)[1] = *((unsigned char *)
-				(bh->b_data + *offset));
+			((unsigned char *) &temp)[1] = data[*offset];
 		else
-			((unsigned char *) &temp)[0] = *((unsigned char *)
-				(bh->b_data + *offset));
+			((unsigned char *) &temp)[0] = data[*offset];
 		brelse(bh);
 		if (!(bh = sb_bread(s, ++(*cur_index))))
 			return NULL;
+		data = bh->b_data;
 		if (msBlk->swap)
-			((unsigned char *) &temp)[0] = *((unsigned char *)
-				bh->b_data); 
+			((unsigned char *) &temp)[0] = data[0];
 		else
-			((unsigned char *) &temp)[1] = *((unsigned char *)
-				bh->b_data); 
+			((unsigned char *) &temp)[1] = data[0];
 		*c_byte = temp;
 		*offset = 1;
 	} else {
+		data = bh->b_data;
 		if (msBlk->swap) {
-			((unsigned char *) &temp)[1] = *((unsigned char *)
-				(bh->b_data + *offset));
-			((unsigned char *) &temp)[0] = *((unsigned char *)
-				(bh->b_data + *offset + 1)); 
+			((unsigned char *) &temp)[1] = data[*offset];
+			((unsigned char *) &temp)[0] = data[*offset + 1]; 
 		} else {
-			((unsigned char *) &temp)[0] = *((unsigned char *)
-				(bh->b_data + *offset));
-			((unsigned char *) &temp)[1] = *((unsigned char *)
-				(bh->b_data + *offset + 1)); 
+			((unsigned char *) &temp)[0] = data[*offset];
+			((unsigned char *) &temp)[1] = data[*offset + 1]; 
 		}
 		*c_byte = temp;
 		*offset += 2;
@@ -169,8 +165,8 @@ static struct buffer_head *get_block_len
 				return NULL;
 			*offset = 0;
 		}
-		if (*((unsigned char *) (bh->b_data + *offset)) !=
-						SQUASHFS_MARKER_BYTE) {
+		data = bh->b_data;
+		if (data[*offset] != SQUASHFS_MARKER_BYTE) {
 			ERROR("Metadata block marker corrupt @ %x\n",
 						*cur_index);
 			brelse(bh);

[PATCH 9/10] squashfs: first use of swabbing

[Jörn Engel]

Jörn

-- 
To recognize individual spam features you have to try to get into the
mind of the spammer, and frankly I want to spend as little time inside
the minds of spammers as possible.
-- Paul Graham


Signed-off-by: Jörn Engel <joern@wohnheim.fh-wedel.de>
---

 fs/squashfs/inode.c |   25 +++++++------------------
 1 files changed, 7 insertions(+), 18 deletions(-)

--- linux-2.6.12-rc3cow/fs/squashfs/inode.c~squashfs_cu10	2005-04-22 09:06:09.550109088 +0200
+++ linux-2.6.12-rc3cow/fs/squashfs/inode.c	2005-04-22 09:15:50.279824752 +0200
@@ -122,7 +122,7 @@ static struct buffer_head *get_block_len
 				int *cur_index, int *offset, int *c_byte)
 {
 	squashfs_sb_info *msBlk = s->s_fs_info;
-	unsigned short temp;
+	u16 temp;
 	struct buffer_head *bh;
 	unsigned char *data;
 
@@ -131,30 +131,19 @@ static struct buffer_head *get_block_len
 
 	if (msBlk->devblksize - *offset == 1) {
 		data = bh->b_data;
-		if (msBlk->swap)
-			((unsigned char *) &temp)[1] = data[*offset];
-		else
-			((unsigned char *) &temp)[0] = data[*offset];
+		((unsigned char *) &temp)[0] = data[*offset];
 		brelse(bh);
 		if (!(bh = sb_bread(s, ++(*cur_index))))
 			return NULL;
 		data = bh->b_data;
-		if (msBlk->swap)
-			((unsigned char *) &temp)[0] = data[0];
-		else
-			((unsigned char *) &temp)[1] = data[0];
-		*c_byte = temp;
+		((unsigned char *) &temp)[1] = data[0];
+		*c_byte = (msBlk->swab16)(temp);
 		*offset = 1;
 	} else {
 		data = bh->b_data;
-		if (msBlk->swap) {
-			((unsigned char *) &temp)[1] = data[*offset];
-			((unsigned char *) &temp)[0] = data[*offset + 1]; 
-		} else {
-			((unsigned char *) &temp)[0] = data[*offset];
-			((unsigned char *) &temp)[1] = data[*offset + 1]; 
-		}
-		*c_byte = temp;
+		((unsigned char *) &temp)[0] = data[*offset];
+		((unsigned char *) &temp)[1] = data[*offset + 1]; 
+		*c_byte = (msBlk->swab16)(temp);
 		*offset += 2;
 	}
 

Re: [PATCH 6/10] squashfs: remove one cast

[Jörn Engel]

On Fri, 22 April 2005 09:22:51 +0200, Jörn Engel wrote:
> Subject: [PATCH 6/10] squashfs: remove one cast

As you should have noticed, patches were sent manually.  That should
have been 7/10.

Jörn

-- 
He that composes himself is wiser than he that composes a book.
-- B. Franklin

Re: [PATCH 8/10] squashfs: add swabbing infrastructure

[Jörn Engel]

On Fri, 22 April 2005 09:23:56 +0200, Jörn Engel wrote:
> +
> +#define SQUASHFS_SWAB(XX)					\
> +u##XX squashfs_swab##XX(u##XX x)	{ return swab##XX(x); }	\
> +u##XX squashfs_ident##XX(u##XX x)	{ return x; }
> +
> +SQUASHFS_SWAB(16)
> +SQUASHFS_SWAB(32)
> +SQUASHFS_SWAB(64)
> +
> +

While it made one function somewhat nicer, I'm not entirely sure this
approach is a good idea in general.  With all the bitfields used,
simple byte-swabbing just doesn't cut it.  Really, the best strategy I
can think of would still be considered A Mess(tm).

It may be time to create a new version of the on-medium layout of the
filesystem.  The new layout is explicitly made big-endian (or
little-endian, whatever) and all structs consist exclusively of u8,
u16, u32 and u64, or their leXX or beXX counterparts.

Advantages:
Swapping is trivially done by beXX_to_cpu and friends.
Code size should decrease significantly.

Disadvantages:
New, incompatible layout.  FS compatibility and code shrinkage are
mutually exclusive.  The thing should be called squashfs2 and have a
completely different code base.
'Bitfield'-access is done by explicit masking and shifting.

Again, this option looks just as rotten as all the previous
alternatives.  The decision is up to you, Phillip.

Jörn

-- 
They laughed at Galileo.  They laughed at Copernicus.  They laughed at
Columbus. But remember, they also laughed at Bozo the Clown.
-- unknown