malloc chunk info in region

2014-04-14 Thread Ted Unangst 
Small tweak. Use a union, instead of casts. There's still casting for
the call to insert(), but I think this is a little better. Also use
the correct type for the insert() parameter.

Index: stdlib/malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.152
diff -u -p -r1.152 malloc.c
--- stdlib/malloc.c 3 Apr 2014 16:18:11 -   1.152
+++ stdlib/malloc.c 14 Apr 2014 17:37:21 -
@@ -94,7 +94,10 @@
 
 struct region_info {
void *p;/* page; low bits used to mark chunks */
-   uintptr_t size; /* size for pages, or chunk_info pointer */
+   union {
+   size_t size;/* size for pages */
+   struct chunk_info *info;/* chunk_info pointer */
+   };
 #ifdef MALLOC_STATS
void *f;/* where allocated from */
 #endif
@@ -737,7 +740,7 @@ alloc_chunk_info(struct dir_info *d, int
  * non-MAP_FIXED mappings with hint 0 start at BRKSIZ.
  */
 static int
-insert(struct dir_info *d, void *p, size_t sz, void *f)
+insert(struct dir_info *d, void *p, uintptr_t sz, void *f)
 {
size_t index;
size_t mask;
@@ -985,7 +988,7 @@ free_bytes(struct dir_info *d, struct re
struct chunk_info *info;
int i;
 
-   info = (struct chunk_info *)r-size;
+   info = r-info;
if (info-canary != d-canary1)
wrterror(chunk info corrupted, NULL);

Re: malloc chunk info in region

2014-04-14 Thread Bob Beck 
On Mon, Apr 14, 2014 at 11:39 AM, Ted Unangst t...@tedunangst.com wrote:
 Small tweak. Use a union, instead of casts. There's still casting for
 the call to insert(), but I think this is a little better. Also use
 the correct type for the insert() parameter.

 Index: stdlib/malloc.c
 ===
 RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
 retrieving revision 1.152
 diff -u -p -r1.152 malloc.c
 --- stdlib/malloc.c 3 Apr 2014 16:18:11 -   1.152
 +++ stdlib/malloc.c 14 Apr 2014 17:37:21 -
 @@ -94,7 +94,10 @@

  struct region_info {
 void *p;/* page; low bits used to mark chunks */
 -   uintptr_t size; /* size for pages, or chunk_info pointer */
 +   union {
 +   size_t size;/* size for pages */
 +   struct chunk_info *info;/* chunk_info pointer */
 +   };
  #ifdef MALLOC_STATS
 void *f;/* where allocated from */
  #endif
 @@ -737,7 +740,7 @@ alloc_chunk_info(struct dir_info *d, int
   * non-MAP_FIXED mappings with hint 0 start at BRKSIZ.
   */
  static int
 -insert(struct dir_info *d, void *p, size_t sz, void *f)
 +insert(struct dir_info *d, void *p, uintptr_t sz, void *f)

Doesn't it make sense for sz to stay a size_t in this case? it appears
to still be used as an offset in here, not a pointer. no?

  {
 size_t index;
 size_t mask;
 @@ -985,7 +988,7 @@ free_bytes(struct dir_info *d, struct re
 struct chunk_info *info;
 int i;

 -   info = (struct chunk_info *)r-size;
 +   info = r-info;
 if (info-canary != d-canary1)
 wrterror(chunk info corrupted, NULL);

Re: malloc chunk info in region

2014-04-14 Thread Otto Moerbeek 
On Mon, Apr 14, 2014 at 11:54:32AM -0600, Bob Beck wrote:

 On Mon, Apr 14, 2014 at 11:39 AM, Ted Unangst t...@tedunangst.com wrote:
  Small tweak. Use a union, instead of casts. There's still casting for
  the call to insert(), but I think this is a little better. Also use
  the correct type for the insert() parameter.
 
  Index: stdlib/malloc.c
  ===
  RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
  retrieving revision 1.152
  diff -u -p -r1.152 malloc.c
  --- stdlib/malloc.c 3 Apr 2014 16:18:11 -   1.152
  +++ stdlib/malloc.c 14 Apr 2014 17:37:21 -
  @@ -94,7 +94,10 @@
 
   struct region_info {
  void *p;/* page; low bits used to mark chunks */
  -   uintptr_t size; /* size for pages, or chunk_info pointer */
  +   union {
  +   size_t size;/* size for pages */
  +   struct chunk_info *info;/* chunk_info pointer */
  +   };
   #ifdef MALLOC_STATS
  void *f;/* where allocated from */
   #endif
  @@ -737,7 +740,7 @@ alloc_chunk_info(struct dir_info *d, int
* non-MAP_FIXED mappings with hint 0 start at BRKSIZ.
*/
   static int
  -insert(struct dir_info *d, void *p, size_t sz, void *f)
  +insert(struct dir_info *d, void *p, uintptr_t sz, void *f)
 
 Doesn't it make sense for sz to stay a size_t in this case? it appears
 to still be used as an offset in here, not a pointer. no?

Indeed, I don't see why you would change the type of the parameter here.

-Otto

 
   {
  size_t index;
  size_t mask;
  @@ -985,7 +988,7 @@ free_bytes(struct dir_info *d, struct re
  struct chunk_info *info;
  int i;
 
  -   info = (struct chunk_info *)r-size;
  +   info = r-info;
  if (info-canary != d-canary1)
  wrterror(chunk info corrupted, NULL);

Re: malloc chunk info in region

2014-04-14 Thread Ted Unangst 
On Mon, Apr 14, 2014 at 20:09, Otto Moerbeek wrote:
   static int
  -insert(struct dir_info *d, void *p, size_t sz, void *f)
  +insert(struct dir_info *d, void *p, uintptr_t sz, void *f)
 
 Doesn't it make sense for sz to stay a size_t in this case? it appears
 to still be used as an offset in here, not a pointer. no?
 
 Indeed, I don't see why you would change the type of the parameter here.

That is what the caller (with a chunk_info *) is casting it to.

Re: malloc chunk info in region

2014-04-14 Thread Otto Moerbeek 
On Mon, Apr 14, 2014 at 02:21:27PM -0400, Ted Unangst wrote:

 On Mon, Apr 14, 2014 at 20:09, Otto Moerbeek wrote:
static int
   -insert(struct dir_info *d, void *p, size_t sz, void *f)
   +insert(struct dir_info *d, void *p, uintptr_t sz, void *f)
  
  Doesn't it make sense for sz to stay a size_t in this case? it appears
  to still be used as an offset in here, not a pointer. no?
  
  Indeed, I don't see why you would change the type of the parameter here.
 
 That is what the caller (with a chunk_info *) is casting it to.

there are two other calls with a size_t arg, and inside insert() the
arg is directly stuffed into a field of the union which is size_t.

-Otto