libaal patch fixing pointer cast warnings

libaal patch fixing pointer cast warnings

[Frederik Himpe]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

When compiling aalib 1.0.5 on x86_64 with gcc 4.3.2 and --enable-Werror,
compilation fails with messages like these:

cc1: warnings being treated as errors
malloc.c: In function '__chunk_free':
malloc.c:164: error: cast from pointer to integer of different size

An updated patch based on a patch from Mandriva is attached here. Could
you review the patch for its correctness and apply it if it's correct?

Thanks,

-- 
Frederik Himpe

[-- Attachment #2: libaal.castint.patch --]
[-- Type: text/x-patch, Size: 1370 bytes --]

diff -ur libaal-1.0.5.orig/src/malloc.c libaal-1.0.5/src/malloc.c
--- libaal-1.0.5.orig/src/malloc.c	2005-07-28 21:13:41.000000000 +0200
+++ libaal-1.0.5/src/malloc.c	2009-02-11 21:14:56.000000000 +0100
@@ -3,6 +3,7 @@
    
    malloc.c -- hanlders for memory allocation functions. */
 
+#include <stdint.h>
 #include <aal/libaal.h>
 
 /* Checking whether allone mode is in use. If so, initializes memory working
@@ -105,8 +106,8 @@
 
 	s = chunk->len - size - sizeof(chunk_t);
 	new = forw ? 
-		(char *)chunk + sizeof(chunk_t) + size :
-		(char *)chunk + sizeof(chunk_t) + s;
+		(void *)(intptr_t)chunk + sizeof(chunk_t) + size :
+		(void *)(intptr_t)chunk + sizeof(chunk_t) + s;
 	
 	/* Okay, we have found chunk good enough. And now we split it onto two
 	   chunks. */
@@ -121,8 +122,8 @@
 
 	area_free -= (size + sizeof(chunk_t));
 	return forw ?
-		(char *)chunk + sizeof(chunk_t):
-		(char *)new + sizeof(chunk_t);
+		(void *)(intptr_t)chunk + sizeof(chunk_t):
+		(void *)(intptr_t)new + sizeof(chunk_t);
 }
 
 /* Makes search for proper memory chunk in list of chunks. If found, split it in
@@ -157,7 +158,7 @@
 }
 
 #define ptr2chunk(ptr) \
-        ((chunk_t *)((int)ptr - sizeof(chunk_t)))
+        ((chunk_t *)((intptr_t)ptr - sizeof(chunk_t)))
 
 /* Frees passed memory pointer */
 static void __chunk_free(void *ptr) {
Only in libaal-1.0.5/src: malloc.c~

Re: libaal patch fixing pointer cast warnings

[Frederik Himpe]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

On wo, 2009-02-11 at 21:23 +0100, Frederik Himpe wrote:
> When compiling aalib 1.0.5 on x86_64 with gcc 4.3.2 and --enable-Werror,
> compilation fails with messages like these:
> 
> cc1: warnings being treated as errors
> malloc.c: In function '__chunk_free':
> malloc.c:164: error: cast from pointer to integer of different size
> 
> An updated patch based on a patch from Mandriva is attached here. Could
> you review the patch for its correctness and apply it if it's correct?

A Mandriva developer checked the patch and made a suggestion, so here is
an improved patch.

-- 
Frederik Himpe

[-- Attachment #2: libaal.castint.patch --]
[-- Type: text/x-patch, Size: 1326 bytes --]

diff -ur libaal-1.0.5.orig/src/malloc.c libaal-1.0.5/src/malloc.c
--- libaal-1.0.5.orig/src/malloc.c	2005-07-28 21:13:41.000000000 +0200
+++ libaal-1.0.5/src/malloc.c	2009-02-11 21:14:56.000000000 +0100
@@ -3,6 +3,7 @@
    
    malloc.c -- hanlders for memory allocation functions. */
 
+#include <stdint.h>
 #include <aal/libaal.h>
 
 /* Checking whether allone mode is in use. If so, initializes memory working
@@ -105,8 +106,8 @@
 
 	s = chunk->len - size - sizeof(chunk_t);
 	new = forw ? 
-		(char *)chunk + sizeof(chunk_t) + size :
-		(char *)chunk + sizeof(chunk_t) + s;
+		(void *)(char *)chunk + sizeof(chunk_t) + size :
+		(void *)(char *)chunk + sizeof(chunk_t) + s;
 	
 	/* Okay, we have found chunk good enough. And now we split it onto two
 	   chunks. */
@@ -121,8 +122,8 @@
 
 	area_free -= (size + sizeof(chunk_t));
 	return forw ?
-		(char *)chunk + sizeof(chunk_t):
-		(char *)new + sizeof(chunk_t);
+		(void *)(char *)chunk + sizeof(chunk_t):
+		(void *)(char *)new + sizeof(chunk_t);
 }
 
 /* Makes search for proper memory chunk in list of chunks. If found, split it in
@@ -157,7 +158,7 @@
 }
 
 #define ptr2chunk(ptr) \
-        ((chunk_t *)((int)ptr - sizeof(chunk_t)))
+        ((chunk_t *)((intptr_t)ptr - sizeof(chunk_t)))
 
 /* Frees passed memory pointer */
 static void __chunk_free(void *ptr) {

Reiser4 Fragmentation

[geearf]

Hello people,

after running into big disk IO I investigated a bit and found that my / is a bit fragmented (the tree is at about 9% from measurefs).
Can that affect a ccreg40 filesystem badly? Anything else relative to R4 that I can look into?


Thank you,
John

Re: Reiser4 Fragmentation

[Edward Shishkin]

geearf@free.fr wrote:
> Hello people,
>   
Hello.
> after running into big disk IO I investigated a bit and found that my / is a bit fragmented (the tree is at about 9% from measurefs).
>   

measurefs.reiser4 is pretty ancient, and I guess
that it works incorrectly for compressed files.

> Can that affect a ccreg40 filesystem badly? Anything else relative to R4 that I can look into?
>   

(External) fragmentation resulted in a number of "extents" is
a sticky thing inherent to all linux file systems, including reiser4.
Currently there is no tools to reorganize reiser4 without having
a spare partition. However, it is possible to create something
simple like xfs_fsr: it would be an appreciable help.

The "smart reiser4 repacker" based on the flush algorithm was
supposed to be the paid option, and therefore there are some
obvious issues of its development.

Internal fragmentation, resulted in wasteful space utilization
("holes" in the metadata blocks") would mean a bug in the code.
This fragmentation can be measured as 1 minus  the fraction
du (1) / df (1). Note, that du (1) should be applied to the semantic
root. If you have ccreg40 partition, then check it with
"fsck.reiser4 --fix" prior using du (1). Also make sure (for example
with sync (1)) that there is no dirty pages prior using df (1).
If the internal fragmentation is more then 0.02, while du (1) is
large enough (> 100 000), then, please, report.

Thanks,
Edward.

Re: libaal patch fixing pointer cast warnings

[Edward Shishkin]

Frederik Himpe writes:
 > On wo, 2009-02-11 at 21:23 +0100, Frederik Himpe wrote:
 > > When compiling aalib 1.0.5 on x86_64 with gcc 4.3.2 and --enable-Werror,
 > > compilation fails with messages like these:
 > > 
 > > cc1: warnings being treated as errors
 > > malloc.c: In function '__chunk_free':
 > > malloc.c:164: error: cast from pointer to integer of different size
 > > 
 > > An updated patch based on a patch from Mandriva is attached here. Could
 > > you review the patch for its correctness and apply it if it's correct?
 > 
 > A Mandriva developer checked the patch and made a suggestion, so here is
 > an improved patch.
 > 
 > -- 
 > Frederik Himpe
 > diff -ur libaal-1.0.5.orig/src/malloc.c libaal-1.0.5/src/malloc.c
 > --- libaal-1.0.5.orig/src/malloc.c	2005-07-28 21:13:41.000000000 +0200
 > +++ libaal-1.0.5/src/malloc.c	2009-02-11 21:14:56.000000000 +0100
 > @@ -3,6 +3,7 @@
 >     
 >     malloc.c -- hanlders for memory allocation functions. */
 >  
 > +#include <stdint.h>
 >  #include <aal/libaal.h>
 >  
 >  /* Checking whether allone mode is in use. If so, initializes memory working
 > @@ -105,8 +106,8 @@
 >  
 >  	s = chunk->len - size - sizeof(chunk_t);
 >  	new = forw ? 
 > -		(char *)chunk + sizeof(chunk_t) + size :
 > -		(char *)chunk + sizeof(chunk_t) + s;
 > +		(void *)(char *)chunk + sizeof(chunk_t) + size :
 > +		(void *)(char *)chunk + sizeof(chunk_t) + s;
 >  	
 >  	/* Okay, we have found chunk good enough. And now we split it onto two
 >  	   chunks. */
 > @@ -121,8 +122,8 @@
 >  
 >  	area_free -= (size + sizeof(chunk_t));
 >  	return forw ?
 > -		(char *)chunk + sizeof(chunk_t):
 > -		(char *)new + sizeof(chunk_t);
 > +		(void *)(char *)chunk + sizeof(chunk_t):
 > +		(void *)(char *)new + sizeof(chunk_t);
 >  }
 >  
 >  /* Makes search for proper memory chunk in list of chunks. If found, split it in
 > @@ -157,7 +158,7 @@
 >  }
 >  
 >  #define ptr2chunk(ptr) \
 > -        ((chunk_t *)((int)ptr - sizeof(chunk_t)))
 > +        ((chunk_t *)((intptr_t)ptr - sizeof(chunk_t)))

Nop, This is wrong!

This macro is to cut a chunk header.
If you cast from pointer to pointer, then it will cut
unexpected number of bytes:
(int)x - y is not the same as (int *)x - y. 

Please, try the following patch.

Edward.

Fix cast.

Signed-off-by: Edward Shishkin<edward.shishkin@gmail.com>
---
 src/malloc.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

--- libaal-1.0.5.orig/src/malloc.c
+++ libaal-1.0.5/src/malloc.c
@@ -105,8 +105,8 @@ static void *__chunk_split(chunk_t *chun
 
 	s = chunk->len - size - sizeof(chunk_t);
 	new = forw ? 
-		(char *)chunk + sizeof(chunk_t) + size :
-		(char *)chunk + sizeof(chunk_t) + s;
+		(void *)chunk + sizeof(chunk_t) + size :
+		(void *)chunk + sizeof(chunk_t) + s;
 	
 	/* Okay, we have found chunk good enough. And now we split it onto two
 	   chunks. */
@@ -121,8 +121,8 @@ static void *__chunk_split(chunk_t *chun
 
 	area_free -= (size + sizeof(chunk_t));
 	return forw ?
-		(char *)chunk + sizeof(chunk_t):
-		(char *)new + sizeof(chunk_t);
+		(void *)chunk + sizeof(chunk_t):
+		(void *)new + sizeof(chunk_t);
 }
 
 /* Makes search for proper memory chunk in list of chunks. If found, split it in
@@ -157,7 +157,7 @@ static void *__chunk_alloc(uint32_t size
 }
 
 #define ptr2chunk(ptr) \
-        ((chunk_t *)((int)ptr - sizeof(chunk_t)))
+        ((chunk_t *)((long)ptr - sizeof(chunk_t)))
 
 /* Frees passed memory pointer */
 static void __chunk_free(void *ptr) {

Re: libaal patch fixing pointer cast warnings

[Frederik Himpe]

On zo, 2009-02-15 at 03:18 +0300, Edward Shishkin wrote:

> Nop, This is wrong!
> 
> This macro is to cut a chunk header.
> If you cast from pointer to pointer, then it will cut
> unexpected number of bytes:
> (int)x - y is not the same as (int *)x - y. 
> 
> Please, try the following patch.

Thanks, it builds fine with your patch.

-- 
Frederik Himpe

Re: Reiser4 Fragmentation

[geearf]

> "Edward Shishkin" <edward.shishkin@gmail.com> wrote:

> Hello.

> Perhaps a simple script is useful, but it has a number of
> disadvantages, which make it a software of "not enterprise" level ;)
> 1) it doesn't take care of holes;
> 2) it doesn't report about cases when reorganization makes things worser.

> Both (1) and (2) need active support of kernel. As to "xfs_fsr simplicity":
> it seems, I have misled you. Xfs provides special ioctls to show/allocate
> "in place" map of extents, whereas reiser4 doesn't have such service.

> Edward.
Oh I see, this looks like something too difficult for me to help with :(
Thanks for the explanation Edward.

John

Re: Reiser4 Fragmentation

[Edward Shishkin]

geearf@free.fr wrote:
>> measurefs.reiser4 is pretty ancient, and I guess
>> that it works incorrectly for compressed files.
>>
>>     
>> (External) fragmentation resulted in a number of "extents" is
>> a sticky thing inherent to all linux file systems, including reiser4.
>> Currently there is no tools to reorganize reiser4 without having
>> a spare partition. However, it is possible to create something
>> simple like xfs_fsr: it would be an appreciable help.
>>     
>> The "smart reiser4 repacker" based on the flush algorithm was
>> supposed to be the paid option, and therefore there are some
>> obvious issues of its development.
> Well I ended up taring my /, mkfs and untaring it.
> So it should not be fragmented in any way anymore.
> I will look at the internal fragmentation of my /home, but this is not under ccreg40.
>
> I've also just looked at the xfs_fsr thing and it looks pretty useful.
> Can't that be done with a simple script? (sort of like Con's defrag script) or does it need to be related to R4 ?
>
>   

Hello.

Perhaps a simple script is useful, but it has a number of
disadvantages, which make it a software of "not enterprise" level ;)
1) it doesn't take care of holes;
2) it doesn't report about cases when reorganization makes things worser.

Both (1) and (2) need active support of kernel. As to "xfs_fsr simplicity":
it seems, I have misled you. Xfs provides special ioctls to show/allocate
"in place" map of extents, whereas reiser4 doesn't have such service.

Edward.

Re: Reiser4 Fragmentation

[geearf]

----- Mail Original -----
"Edward Shishkin" <edward.shishkin@gmail.com> wrote:
>geearf@free.fr wrote:
>Hello.

>measurefs.reiser4 is pretty ancient, and I guess
>that it works incorrectly for compressed files.


>(External) fragmentation resulted in a number of "extents" is
>a sticky thing inherent to all linux file systems, including reiser4.
>Currently there is no tools to reorganize reiser4 without having
>a spare partition. However, it is possible to create something
>simple like xfs_fsr: it would be an appreciable help.

>The "smart reiser4 repacker" based on the flush algorithm was
>supposed to be the paid option, and therefore there are some
>obvious issues of its development.

>Internal fragmentation, resulted in wasteful space utilization
>("holes" in the metadata blocks") would mean a bug in the code.
>This fragmentation can be measured as 1 minus  the fraction
>du (1) / df (1). Note, that du (1) should be applied to the semantic
>root. If you have ccreg40 partition, then check it with
>"fsck.reiser4 --fix" prior using du (1). Also make sure (for example
>with sync (1)) that there is no dirty pages prior using df (1).
>If the internal fragmentation is more then 0.02, while du (1) is
>large enough (> 100 000), then, please, report.

>Thanks,
>Edward.

Well I ended up taring my /, mkfs and untaring it.
So it should not be fragmented in any way anymore.
I will look at the internal fragmentation of my /home, but this is not under ccreg40.

I've also just looked at the xfs_fsr thing and it looks pretty useful.
Can't that be done with a simple script? (sort of like Con's defrag script) or does it need to be related to R4 ?

Thanks Edward,
John