Re: [ros-dev] [ros-diffs] [greatlrd] 22195: make malloc, calloc and some other function return NULL if size is 0

[Royce Mitchell III]

[e-mail] wrote:
> @@ -52,6 +55,9 @@
>  */
> void* calloc(size_t _nmemb, size_t _size)
> {
> if ( _size == 0)
>   return NULL;
>      
> return HeapAlloc(hHeap, HEAP_ZERO_MEMORY, ROUND_SIZE(_nmemb*_size) );
>
> }

What if _nmemb is zero?
> @@ -60,6 +66,9 @@
>  */
> void* realloc(void* _ptr, size_t _size)
> {
> if ( _size == 0)
>   return NULL;
>      
> if (!_ptr) return malloc(_size);
>  if (_size) return HeapReAlloc(hHeap, 0, _ptr, ROUND_SIZE(_size));
>  free(_ptr);
>
> @@ -71,6 +80,9 @@
>  */

umm... shouldn't we call free() if we're going to return NULL because 
size is 0?

Also, the free() at the bottom is never called now.
> void* _expand(void* _ptr, size_t _size)
> {
> if ( _size == 0)
>   return NULL;
>      
> return HeapReAlloc(hHeap, HEAP_REALLOC_IN_PLACE_ONLY, _ptr, ROUND_SIZE(_size));
>
> }

Again, shouldn't we free the existing buffer if we're returning NULL 
because size is 0?

[Thomas Weidenmueller]

Royce Mitchell III wrote:
> > @@ -60,6 +66,9 @@
> >  */
> > void* realloc(void* _ptr, size_t _size)
> > {
> > if ( _size == 0)
> >   return NULL;
> >      
> > if (!_ptr) return malloc(_size);
> >  if (_size) return HeapReAlloc(hHeap, 0, _ptr, ROUND_SIZE(_size));
> >  free(_ptr);
> >
> > @@ -71,6 +80,9 @@
> >  */
>
> umm... shouldn't we call free() if we're going to return NULL because 
> size is 0?

realloc never frees the memory if it fails.
> Also, the free() at the bottom is never called now.

it's wrong and shouldn't even be there ;)
> > void* _expand(void* _ptr, size_t _size)
> > {
> > if ( _size == 0)
> >   return NULL;
> >      
> > return HeapReAlloc(hHeap, HEAP_REALLOC_IN_PLACE_ONLY, _ptr, ROUND_SIZE(_size));
> >
> > }
>
> Again, shouldn't we free the existing buffer if we're returning NULL 
> because size is 0?

no, same as with realloc.

- Thomas

[Magnus Olsen]

Maybe we should add free to realloc and expand
to avoid memmory leak ??

I know realloc fail in windows it can create memory leak

text from internet how malloc should work

" If the size of the space requested is 0, the behavior is
implementation-defined:
  the value returned shall be either a null pointer or a unique pointer."

But most devloper assume malloc will return NULL if size is 0
 But some windows platfroms it return vaild pointer.

The question is how should we handle it in reactos,
return NULL or vaild pointer  for this case ?

----- Original Message ----- 
From: "Thomas Weidenmueller" [e-mail]
To: "ReactOS Development List" [e-mail]
Sent: Saturday, June 03, 2006 4:39 PM
Subject: Re: [ros-dev] [ros-diffs] [greatlrd] 22195: make malloc, calloc and
some other function return NULL if size is 0
> Royce Mitchell III wrote:
> > > @@ -60,6 +66,9 @@
> > >  */
> > > void* realloc(void* _ptr, size_t _size)
> > > {
> > > if ( _size == 0)
> > >   return NULL;
> > > if (!_ptr) return malloc(_size);
> > >  if (_size) return HeapReAlloc(hHeap, 0, _ptr, ROUND_SIZE(_size));
> > >  free(_ptr);
> > >
> > > @@ -71,6 +80,9 @@
> > >  */
> >
> > umm... shouldn't we call free() if we're going to return NULL because
> > size is 0?
>
> realloc never frees the memory if it fails.
> > Also, the free() at the bottom is never called now.
>
> it's wrong and shouldn't even be there ;)
> > > void* _expand(void* _ptr, size_t _size)
> > > {
> > > if ( _size == 0)
> > >   return NULL;
> > > return HeapReAlloc(hHeap, HEAP_REALLOC_IN_PLACE_ONLY, _ptr,

ROUND_SIZE(_size));
> > > }
> >
> > Again, shouldn't we free the existing buffer if we're returning NULL
> > because size is 0?
>
> no, same as with realloc.
> Thomas
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Gunnar Dalsnes]

Was the changes made to fix something? If not: don't fix whats not broken.

MSDN: The return value is NULL if the size is zero and the buffer 
argument is not NULL, or if there is not enough available memory to 
expand the block to the given size. In the first case, the original 
block is freed. In the second, the original block is unchanged.

Magnus Olsen wrote:
> Maybe we should add free to realloc and expand
> to avoid memmory leak ??

NO!!!!!!!!!!!!! MSDN says the memory will not be freed if the call 
fails, so the caller is free to continue to use the memory for whatever 
he likes.
> I know realloc fail in windows it can create memory leak

MSDN: The return value is NULL if the size is zero and the buffer 
argument is not NULL, or if there is not enough available memory to 
expand the block to the given size. In the first case, the original 
block is freed. In the second, the original block is unchanged.

So, if realloc fails in windows it WILL ALWAYS create a memory leak 
(unless you free the memory urself offcourse:-)
> text from internet how malloc should work

With Internet do you mean MSDN? Every implementation of malloc behave 
different. We only want it to behave like the Windows version...
> " If the size of the space requested is 0, the behavior is
> implementation-defined:
>   the value returned shall be either a null pointer or a unique pointer."

MSDN: If size is 0, malloc allocates a zero-length item in the heap and 
returns a valid pointer to that item.
> But most devloper assume malloc will return NULL if size is 0
>  But some windows platfroms it return vaild pointer.
>
> The question is how should we handle it in reactos,
> return NULL or vaild pointer  for this case ?

Don't try to be smart, just do it like it says on MSDN.

BTW: The original code looks correct to me.
> ----- Original Message ----- 
> From: "Thomas Weidenmueller" [e-mail]
> To: "ReactOS Development List" [e-mail]
> Sent: Saturday, June 03, 2006 4:39 PM
> Subject: Re: [ros-dev] [ros-diffs] [greatlrd] 22195: make malloc, calloc and
> some other function return NULL if size is 0
> > Royce Mitchell III wrote:
> > > > @@ -60,6 +66,9 @@
> > > >  */
> > > > void* realloc(void* _ptr, size_t _size)
> > > > {
> > > > if ( _size == 0)
> > > >   return NULL;
> > > > if (!_ptr) return malloc(_size);
> > > >  if (_size) return HeapReAlloc(hHeap, 0, _ptr, ROUND_SIZE(_size));
> > > >  free(_ptr);
> > > >
> > > > @@ -71,6 +80,9 @@
> > > >  */
> > >
> > > umm... shouldn't we call free() if we're going to return NULL because
> > > size is 0?
> >
> > realloc never frees the memory if it fails.
> > > Also, the free() at the bottom is never called now.
> >
> > it's wrong and shouldn't even be there ;)
> > > > void* _expand(void* _ptr, size_t _size)
> > > > {
> > > > if ( _size == 0)
> > > >   return NULL;
> > > > return HeapReAlloc(hHeap, HEAP_REALLOC_IN_PLACE_ONLY, _ptr,
>
> ROUND_SIZE(_size));
> > > > }
> > >
> > > Again, shouldn't we free the existing buffer if we're returning NULL
> > > because size is 0?
> >
> > no, same as with realloc.
> > Thomas
> >
> > Ros-dev mailing list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Magnus Olsen]

HI
Malloc(0) did not reaturn NULL;
that was the fix for. I am reading the IEEE spec
how malloc should work, and it exists two diffent
implement of it. and some exprenet on diffent windows
platfrom show diffent behvior about malloc(0)

----- Original Message ----- 
From: "Gunnar Dalsnes" [e-mail]
To: "ReactOS Development List" [e-mail]
Sent: Saturday, June 03, 2006 6:16 PM
Subject: Re: [ros-dev] [ros-diffs] [greatlrd] 22195: make malloc,calloc and
some other function return NULL if size is 0
> Was the changes made to fix something? If not: don't fix whats not broken.
>
> MSDN: The return value is NULL if the size is zero and the buffer
> argument is not NULL, or if there is not enough available memory to
> expand the block to the given size. In the first case, the original
> block is freed. In the second, the original block is unchanged.
>
> Magnus Olsen wrote:
> > Maybe we should add free to realloc and expand
> > to avoid memmory leak ??
>
> NO!!!!!!!!!!!!! MSDN says the memory will not be freed if the call
> fails, so the caller is free to continue to use the memory for whatever
> he likes.
> > I know realloc fail in windows it can create memory leak
>
> MSDN: The return value is NULL if the size is zero and the buffer
> argument is not NULL, or if there is not enough available memory to
> expand the block to the given size. In the first case, the original
> block is freed. In the second, the original block is unchanged.
>
> So, if realloc fails in windows it WILL ALWAYS create a memory leak
> (unless you free the memory urself offcourse:-)
> > text from internet how malloc should work
>
> With Internet do you mean MSDN? Every implementation of malloc behave
> different. We only want it to behave like the Windows version...
> > " If the size of the space requested is 0, the behavior is
> > implementation-defined:
> >   the value returned shall be either a null pointer or a unique

pointer."
> >
>
> MSDN: If size is 0, malloc allocates a zero-length item in the heap and
> returns a valid pointer to that item.
> > But most devloper assume malloc will return NULL if size is 0
> >  But some windows platfroms it return vaild pointer.
> >
> > The question is how should we handle it in reactos,
> > return NULL or vaild pointer  for this case ?
>
> Don't try to be smart, just do it like it says on MSDN.
>
> BTW: The original code looks correct to me.
> > ----- Original Message ----- 
> > From: "Thomas Weidenmueller" [e-mail]
> > To: "ReactOS Development List" [e-mail]
> > Sent: Saturday, June 03, 2006 4:39 PM
> > Subject: Re: [ros-dev] [ros-diffs] [greatlrd] 22195: make malloc, calloc

and
> > some other function return NULL if size is 0
> > > Royce Mitchell III wrote:
> > > > > @@ -60,6 +66,9 @@
> > > > >  */
> > > > > void* realloc(void* _ptr, size_t _size)
> > > > > {
> > > > > if ( _size == 0)
> > > > >   return NULL;
> > > > > if (!_ptr) return malloc(_size);
> > > > >  if (_size) return HeapReAlloc(hHeap, 0, _ptr, ROUND_SIZE(_size));
> > > > >  free(_ptr);
> > > > >
> > > > > @@ -71,6 +80,9 @@
> > > > >  */
> > > >
> > > > umm... shouldn't we call free() if we're going to return NULL because
> > > > size is 0?
> > >
> > > realloc never frees the memory if it fails.
> > > > Also, the free() at the bottom is never called now.
> > >
> > > it's wrong and shouldn't even be there ;)
> > > > > void* _expand(void* _ptr, size_t _size)
> > > > > {
> > > > > if ( _size == 0)
> > > > >   return NULL;
> > > > > return HeapReAlloc(hHeap, HEAP_REALLOC_IN_PLACE_ONLY, _ptr,
> >
> > ROUND_SIZE(_size));
> > > > > }
> > > >
> > > > Again, shouldn't we free the existing buffer if we're returning NULL
> > > > because size is 0?
> > >
> > > no, same as with realloc.
> > > Thomas
> > >
> > > Ros-dev mailing list
> > > [e-mail]
> > > http://www.reactos.org/mailman/listinfo/ros-dev
> >
> > Ros-dev mailing list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Gunnar Dalsnes]

Magnus Olsen wrote:
> HI
> Malloc(0) did not reaturn NULL;
> that was the fix for. I am reading the IEEE spec
> how malloc should work, and it exists two diffent
> implement of it. and some exprenet on diffent windows
> platfrom show diffent behvior about malloc(0)

IEEE specs really have nothing to do with how Windows behaves. Windows 
NEVER follow standards. A quick test on Win XP shows malloc(0) return a 
pointer to stack, so MSDN is right (surprise!) and your fix is wrong. If 
some older version of Windows did otherwise, this is a job for a 
compability framework (which we dont have).

[James Tabor]

Gunnar Dalsnes wrote:
> Magnus Olsen wrote:
> > HI
> > Malloc(0) did not reaturn NULL;
> > that was the fix for. I am reading the IEEE spec
> > how malloc should work, and it exists two diffent
> > implement of it. and some exprenet on diffent windows
> > platfrom show diffent behvior about malloc(0)
>
> IEEE specs really have nothing to do with how Windows behaves. Windows 
> NEVER follow standards. A quick test on Win XP shows malloc(0) return a 
> pointer to stack, so MSDN is right (surprise!) and your fix is wrong. If 
> some older version of Windows did otherwise, this is a job for a 
> compability framework (which we dont have).

Wow!
what a cheesy way to get the stack pointer.
James

[Magnus Olsen]

I talk with filip about it, he confrim the fix is correct

IEEE spec did mentor it exists two diffent behvoir
on malloc(0). My windows 2000 is return NULL from
malloc(0) and some other windows I got did not.

----- Original Message ----- 
From: "Gunnar Dalsnes" [e-mail]
To: "ReactOS Development List" [e-mail]
Sent: Saturday, June 03, 2006 6:57 PM
Subject: Re: [ros-dev] [ros-diffs] [greatlrd] 22195: make malloc,calloc and
some other function return NULL if size is 0
> Magnus Olsen wrote:
> > HI
> > Malloc(0) did not reaturn NULL;
> > that was the fix for. I am reading the IEEE spec
> > how malloc should work, and it exists two diffent
> > implement of it. and some exprenet on diffent windows
> > platfrom show diffent behvior about malloc(0)
>
> IEEE specs really have nothing to do with how Windows behaves. Windows
> NEVER follow standards. A quick test on Win XP shows malloc(0) return a
> pointer to stack, so MSDN is right (surprise!) and your fix is wrong. If
> some older version of Windows did otherwise, this is a job for a
> compability framework (which we dont have).
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Royce Mitchell III]

Gunnar Dalsnes wrote:
> this is a job for a 
> compability framework (which we dont have).

Umm... we did 3 years ago... was it removed?

[D. Hazelton]

On Saturday 03 June 2006 14:39, Thomas Weidenmueller wrote:
> Royce Mitchell III wrote:
> > > @@ -60,6 +66,9 @@
> > >  */
> > > void* realloc(void* _ptr, size_t _size)
> > > {
> > > if ( _size == 0)
> > >   return NULL;
> > > if (!_ptr) return malloc(_size);
> > >  if (_size) return HeapReAlloc(hHeap, 0, _ptr, ROUND_SIZE(_size));
> > >  free(_ptr);
> > >
> > > @@ -71,6 +80,9 @@
> > >  */
> >
> > umm... shouldn't we call free() if we're going to return NULL because
> > size is 0?
>
> realloc never frees the memory if it fails.

Calling realloc() sith size as 0 is supposed to free the buffer, not fail. 
Heres how realloc() is defined in the man-page:
       realloc()  changes  the  size  of the memory block pointed to by ptr to
       size bytes.  The contents will be unchanged to the minimum of  the  old
       and new sizes; newly allocated memory will be uninitialized.  If ptr is
       NULL, the call is equivalent to malloc(size); if size is equal to zero,
       the  call is equivalent to free(ptr).  Unless ptr is NULL, it must have
       been returned by an earlier call to malloc(),  calloc()  or  realloc().
       If the area pointed to was moved, a free(ptr) is done.
> > > void* _expand(void* _ptr, size_t _size)
> > > {
> > > if ( _size == 0)
> > >   return NULL;
> > > return HeapReAlloc(hHeap, HEAP_REALLOC_IN_PLACE_ONLY, _ptr,
> > >
> > > ROUND_SIZE(_size)); }
> >
> > Again, shouldn't we free the existing buffer if we're returning NULL
> > because size is 0?
>
> no, same as with realloc.

This call isn't in Posix or any C library I've ever used, but seems to be 
correct. Expanding a buffer by 0 bytes, if called from userspace, should 
return the original buffer. Called internally, like this appears to be, 
returning NULL indicates the buffer wasn't touched.

DRH
(If you need help with how people will *expect* known functions to work 
(specifically C library functions) just ask :)

[D. Hazelton]

Sorry about that last post. I forgot that MS never follows published 
specifications. The ANSI C standard states that malloc() of any memory should 
either allocate the memory or fail - most implementations of the standard C 
library return NULL on a call to malloc with 0 as the size.

Calls to realloc() with size as 0 are supposed to free the pointer.

However, MS doesn't follow ANSI at all, anywhere, and since ReactOS is meant 
to be 100% binary compatable with Windows it should follow the MSDN specs for 
functions.

DRH

[Myria]

I always thought that Microsoft's malloc() returned a fully valid heap 
pointer when you pass a size of zero.  Such a pointer would be a valid heap 
pointer of zero size (consisting only of a heap linked list entry "before" 
the pointer), and could be freed with free().

realloc() does free() and returns NULL if _size is zero.  This is even true 
if you originally passed zero to malloc() and got an empty heap pointer.  In 
this way, realloc is inconsistent with malloc(), but that's the way it is.

Melissa

----- Original Message ----- 
From: "D. Hazelton" [e-mail]
To: [e-mail]
Sent: Saturday, June 03, 2006 15:44
Subject: Re: [ros-dev] [ros-diffs] [greatlrd] 22195: make malloc,calloc and 
some other function return NULL if size is 0
> Sorry about that last post. I forgot that MS never follows published
> specifications. The ANSI C standard states that malloc() of any memory 
> should
> either allocate the memory or fail - most implementations of the standard 
> C
> library return NULL on a call to malloc with 0 as the size.
>
> Calls to realloc() with size as 0 are supposed to free the pointer.
>
> However, MS doesn't follow ANSI at all, anywhere, and since ReactOS is 
> meant
> to be 100% binary compatable with Windows it should follow the MSDN specs 
> for
> functions.
>
> DRH
> _______________________________________________
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev