Re: [PATCH] remove some usesless casts

2005-04-21 Thread 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-21 Thread 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 <[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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-21 Thread 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 [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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-21 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-20 Thread 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-20 Thread 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.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-20 Thread 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-20 Thread 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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH] remove some usesless casts

2005-04-20 Thread 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 <[EMAIL PROTECTED]>
---

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

--- linux-2.6.12-rc2cow/fs/squashfs/inode.c~squashfs_cu12005-04-20 
07:52:46.0 +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(>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 = >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 = >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 = >sBlk;

msBlk->devblksize = sb_min_blocksize(s, BLOCK_SIZE);
@@ -914,8 +912,7 @@ static int squashfs_fill_super(struct su
s->s_op = _ops;
 
/* Init inode_table block pointer array */
-   if (!(msBlk->block_cache = (squashfs_cache *)
-   

[PATCH] remove some usesless casts

2005-04-20 Thread 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 [EMAIL PROTECTED]
---

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

--- linux-2.6.12-rc2cow/fs/squashfs/inode.c~squashfs_cu12005-04-20 
07:52:46.0 +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 *)
-   

Re: [PATCH] remove some usesless casts

2005-04-20 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-20 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-20 Thread 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.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH] remove some usesless casts

2005-04-20 Thread 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
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/