Re: Fix kernel heap test

[Dimitrie O. Paun]

On December 15, 2003 10:35 am, Hans Leidekker wrote:
> The test succeeds on Wine. This is wrong of course, tests should
> always succeed on Windows and either succeed on Wine or fail on
> Wine and be marked todo_wine.

Great, thanks for catching this. Our current implementation seems
to be freeing memory on size=0, how can we check what Windows does?
If it doesn't, we can just simpligy the code to this:

Index: dlls/kernel/heap.c
===================================================================
RCS file: /var/cvs/wine/dlls/kernel/heap.c,v
retrieving revision 1.4
diff -u -r1.4 heap.c
--- dlls/kernel/heap.c  27 Nov 2003 00:59:36 -0000      1.4
+++ dlls/kernel/heap.c  15 Dec 2003 16:19:04 -0000
@@ -615,37 +614,15 @@
             SetLastError(ERROR_INVALID_HANDLE);
          } else
 #endif
-         if(size!=0)
-         {
-            hnew=hmem;
-            if(pintern->Pointer)
-            {
-               if((palloc = HeapReAlloc(GetProcessHeap(), heap_flags,
-                                   (char *) pintern->Pointer-HGLOBAL_STORAGE,
-                                   size+HGLOBAL_STORAGE)) == NULL)
-                   hnew = 0; /* Block still valid */
-               else
-                   pintern->Pointer = (char *)palloc+HGLOBAL_STORAGE;
-            }
-            else
-            {
-                if((palloc=HeapAlloc(GetProcessHeap(), heap_flags, size+HGLOBAL_STORAGE))
-                   == NULL)
-                    hnew = 0;
-                else
-                {
-                    *(HGLOBAL *)palloc = hmem;
-                    pintern->Pointer = (char *)palloc + HGLOBAL_STORAGE;
-                }
-            }
-         }
+         if(size == 0) size = 16;
+         hnew=hmem;
+         if(pintern->Pointer) palloc = HeapAlloc(GetProcessHeap(), heap_flags, size+HGLOBAL_STORAGE);
+         else palloc = HeapReAlloc(GetProcessHeap(), heap_flags, (char *) pintern->Pointer-HGLOBAL_STORAGE, size+HGLOBAL_STORAGE);
+         if (!palloc)  hnew = 0;
          else
          {
-            if(pintern->Pointer)
-            {
-               HeapFree(GetProcessHeap(), 0, (char *) pintern->Pointer-HGLOBAL_STORAGE);
-               pintern->Pointer=NULL;
-            }
+             pintern->Pointer = (char *)palloc + HGLOBAL_STORAGE;
+             if (!pintern->Pointer) *(HGLOBAL *)palloc = hmem;
          }
       }
    }

Fix kernel heap test

[Hans Leidekker]

On Monday 15 December 2003 17:20, Dimitrie O. Paun wrote:
> Great, thanks for catching this. Our current implementation seems
> to be freeing memory on size=0, how can we check what Windows does?

On Windows resizing to 0 succeeds, and if you then the check the
size of the memory object you find the same size as before
resizing. I don't know what happens with large resizes (larger
than some internal block unit), haven't checked, but I guess 
rounding to the nearest block multiple is involved here.

Furthermore, if you resize to anything larger than 0, but smaller
than the orginal size you get your smaller size on Win2k but
the original size on Win98. Again, I see this with small resizes,
don't know about large resizes.
> If it doesn't, we can just simpligy the code to this:

Your patch get's rid of the errors I reported, but it introduces
new errors:

heap.c:46: Test failed: Can't realloc global memory
heap.c:48: Test failed: Memory not resized to size 10, instead size=0
heap.c:53: Test failed: GlobalReAlloc should not fail on size 0
heap.c:70: Test failed: Can't realloc local memory
heap.c:72: Test failed: Memory not resized to size 10, instead size=0
heap.c:77: Test failed: LocalReAlloc should not fail on size 0

Worse is that other tests start to fail too:

alloc.c:237: Test failed: GlobalHandle didn't return the correct memory handle
alloc.c:241: Test failed: Discarded memory we shouldn't have
alloc.c:327: Test failed: LocalHandle didn't return the correct memory handle
alloc.c:330: Test failed: Discarded memory we shouldn't have

-Hans