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 <[email protected]>
---
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)
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
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
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.
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
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 <[email protected]>
---
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,
J?rn
--
It's just what we asked for, but not what we want!
-- anonymous
Signed-off-by: J?rn Engel <[email protected]>
---
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);
J?rn
--
Anything that can go wrong, will.
-- Finagle's Law
Signed-off-by: J?rn Engel <[email protected]>
---
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,
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 <[email protected]> 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
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
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 <[email protected]>
---
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
J?rn
--
All art is but imitation of nature.
-- Lucius Annaeus Seneca
Signed-off-by: J?rn Engel <[email protected]>
---
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;
}
J?rn
--
/* Keep these two variables together */
int bar;
Signed-off-by: J?rn Engel <[email protected]>
---
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");
J?rn
--
Correctness comes second.
Features come third.
Performance comes last.
Maintainability is needed for all of them.
Signed-off-by: J?rn Engel <[email protected]>
---
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;
J?rn
--
Ninety percent of everything is crap.
-- Sturgeon's Law
Signed-off-by: J?rn Engel <[email protected]>
---
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);
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 <[email protected]>
---
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;
}
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
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