[PATCH 6/6] kernelbase: Cleanup and simplify (Global|Local)ReAlloc.

Rémi Bernon wine at gitlab.winehq.org
Fri Jun 10 03:19:59 CDT 2022


From: Rémi Bernon <rbernon at codeweavers.com>

Signed-off-by: Rémi Bernon <rbernon at codeweavers.com>
---
 dlls/kernel32/heap.c       |  22 +++++++-
 dlls/kernel32/tests/heap.c |  15 ------
 dlls/kernelbase/memory.c   | 108 +++++++++++++------------------------
 3 files changed, 56 insertions(+), 89 deletions(-)

diff --git a/dlls/kernel32/heap.c b/dlls/kernel32/heap.c
index 3ba9e139171..a2910688e44 100644
--- a/dlls/kernel32/heap.c
+++ b/dlls/kernel32/heap.c
@@ -48,6 +48,7 @@ WINE_DEFAULT_DEBUG_CHANNEL(globalmem);
 static HANDLE systemHeap;   /* globally shared heap */
 
 BOOLEAN WINAPI RtlGetUserInfoHeap( HANDLE handle, ULONG flags, void *ptr, void **user_value, ULONG *user_flags );
+BOOLEAN WINAPI RtlSetUserValueHeap( HANDLE handle, ULONG flags, void *ptr, void *user_value );
 
 /***********************************************************************
  *           HEAP_CreateSystemHeap
@@ -174,7 +175,7 @@ struct kernelbase_global_data *kernelbase_global_data;
 
 static inline struct mem_entry *unsafe_mem_from_HLOCAL( HLOCAL handle )
 {
-    struct mem_entry *mem = CONTAINING_RECORD( handle, struct mem_entry, ptr );
+    struct mem_entry *mem = CONTAINING_RECORD( *(volatile HANDLE *)&handle, struct mem_entry, ptr );
     struct kernelbase_global_data *data = kernelbase_global_data;
     if (((UINT_PTR)handle & ((sizeof(void *) << 1) - 1)) != sizeof(void *)) return NULL;
     if (mem < data->mem_entries || mem >= data->mem_entries_end) return NULL;
@@ -253,7 +254,24 @@ HGLOBAL WINAPI GlobalHandle( const void *ptr )
  */
 HGLOBAL WINAPI GlobalReAlloc( HGLOBAL handle, SIZE_T size, UINT flags )
 {
-    return LocalReAlloc( handle, size, flags );
+    struct mem_entry *mem;
+    void *ptr;
+
+    if ((mem = unsafe_mem_from_HLOCAL( handle )) && mem->lock) return 0;
+    if (!(handle = LocalReAlloc( handle, size, flags ))) return 0;
+
+    /* GlobalReAlloc allows changing GMEM_FIXED to GMEM_MOVEABLE with GMEM_MODIFY */
+    if ((flags & (GMEM_MOVEABLE | GMEM_MODIFY)) == (GMEM_MOVEABLE | GMEM_MODIFY) &&
+        (ptr = unsafe_ptr_from_HLOCAL( handle )))
+    {
+        if (!(handle = LocalAlloc( flags, 0 ))) return 0;
+        RtlSetUserValueHeap( GetProcessHeap(), 0, ptr, handle );
+        mem = unsafe_mem_from_HLOCAL( handle );
+        mem->flags &= ~MEM_FLAG_DISCARDED;
+        mem->ptr = ptr;
+    }
+
+    return handle;
 }
 
 
diff --git a/dlls/kernel32/tests/heap.c b/dlls/kernel32/tests/heap.c
index 4496df35579..90c311b5683 100644
--- a/dlls/kernel32/tests/heap.c
+++ b/dlls/kernel32/tests/heap.c
@@ -1629,9 +1629,7 @@ static void test_GlobalAlloc(void)
     mem = GlobalAlloc( GMEM_FIXED, 10 );
     ok( !!mem, "GlobalAlloc failed, error %lu\n", GetLastError() );
     tmp_mem = GlobalReAlloc( mem, 9, GMEM_MODIFY );
-    todo_wine
     ok( !!tmp_mem, "GlobalAlloc failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "got ptr %p, expected %p\n", tmp_mem, mem );
     size = GlobalSize( mem );
     ok( size == 10, "GlobalSize returned %Iu\n", size );
@@ -1645,9 +1643,7 @@ static void test_GlobalAlloc(void)
         "got error %lu\n", GetLastError() );
     if (tmp_mem) mem = tmp_mem;
     tmp_mem = GlobalReAlloc( mem, 1024 * 1024, GMEM_MODIFY );
-    todo_wine
     ok( !!tmp_mem, "GlobalAlloc failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "got ptr %p, expected %p\n", tmp_mem, mem );
     size = GlobalSize( mem );
     ok( size == 10, "GlobalSize returned %Iu\n", size );
@@ -1986,9 +1982,7 @@ static void test_LocalAlloc(void)
     mem = LocalAlloc( LMEM_FIXED, 10 );
     ok( !!mem, "LocalAlloc failed, error %lu\n", GetLastError() );
     tmp_mem = LocalReAlloc( mem, 9, LMEM_MODIFY );
-    todo_wine
     ok( !!tmp_mem, "LocalAlloc failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "got ptr %p, expected %p\n", tmp_mem, mem );
     size = LocalSize( mem );
     ok( size == 10, "LocalSize returned %Iu\n", size );
@@ -2002,9 +1996,7 @@ static void test_LocalAlloc(void)
         "got error %lu\n", GetLastError() );
     if (tmp_mem) mem = tmp_mem;
     tmp_mem = LocalReAlloc( mem, 1024 * 1024, LMEM_MODIFY );
-    todo_wine
     ok( !!tmp_mem, "LocalAlloc failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "got ptr %p, expected %p\n", tmp_mem, mem );
     size = LocalSize( mem );
     ok( size == 10, "LocalSize returned %Iu\n", size );
@@ -2072,18 +2064,15 @@ static void test_LocalAlloc(void)
     /* Check that we cannot change LMEM_FIXED to LMEM_MOVEABLE */
     mem = LocalReAlloc( mem, 0, LMEM_MODIFY | LMEM_MOVEABLE );
     ok( !!mem, "LocalReAlloc failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( mem == ptr, "LocalReAlloc returned unexpected handle\n" );
     size = LocalSize( mem );
     ok( size == buffer_size, "LocalSize returned %Iu, error %lu\n", size, GetLastError() );
 
     ptr = LocalLock( mem );
     ok( !!ptr, "LocalLock failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( ptr == mem, "got unexpected ptr %p\n", ptr );
     ret = LocalUnlock( mem );
     ok( !ret, "LocalUnlock succeeded, error %lu\n", GetLastError() );
-    todo_wine
     ok( GetLastError() == ERROR_NOT_LOCKED, "got error %lu\n", GetLastError() );
 
     tmp_mem = LocalReAlloc( mem, 2 * buffer_size, LMEM_MOVEABLE | LMEM_ZEROINIT );
@@ -2096,7 +2085,6 @@ static void test_LocalAlloc(void)
     ok( size >= 2 * buffer_size, "LocalSize returned %Iu, error %lu\n", size, GetLastError() );
     ptr = LocalLock( mem );
     ok( !!ptr, "LocalLock failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( ptr == mem, "got unexpected ptr %p\n", ptr );
     ok( !memcmp( ptr, zero_buffer, buffer_size ), "LocalReAlloc didn't clear memory\n" );
     ok( !memcmp( ptr + buffer_size, zero_buffer, buffer_size ),
@@ -2105,13 +2093,10 @@ static void test_LocalAlloc(void)
     tmp_mem = LocalHandle( ptr );
     ok( tmp_mem == mem, "LocalHandle returned unexpected handle\n" );
     tmp_mem = LocalDiscard( mem );
-    todo_wine
     ok( !!tmp_mem, "LocalDiscard failed, error %lu\n", GetLastError() );
-    todo_wine
     ok( tmp_mem == mem, "LocalDiscard returned unexpected handle\n" );
     ret = LocalUnlock( mem );
     ok( !ret, "LocalUnlock succeeded, error %lu\n", GetLastError() );
-    todo_wine
     ok( GetLastError() == ERROR_NOT_LOCKED, "got error %lu\n", GetLastError() );
 
     tmp_mem = LocalDiscard( mem );
diff --git a/dlls/kernelbase/memory.c b/dlls/kernelbase/memory.c
index 16a068f5b5f..e24a6f9aad0 100644
--- a/dlls/kernelbase/memory.c
+++ b/dlls/kernelbase/memory.c
@@ -964,7 +964,8 @@ LPVOID WINAPI DECLSPEC_HOTPATCH LocalLock( HLOCAL handle )
  */
 HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT flags )
 {
-    DWORD heap_flags = HEAP_ADD_USER_INFO;
+    DWORD heap_flags = HEAP_ADD_USER_INFO | HEAP_NO_SERIALIZE;
+    HANDLE heap = GetProcessHeap();
     struct mem_entry *mem;
     HLOCAL ret = 0;
     void *ptr;
@@ -973,92 +974,55 @@ HLOCAL WINAPI DECLSPEC_HOTPATCH LocalReAlloc( HLOCAL handle, SIZE_T size, UINT f
 
     if (flags & LMEM_ZEROINIT) heap_flags |= HEAP_ZERO_MEMORY;
 
-    RtlLockHeap( GetProcessHeap() );
-    if (flags & LMEM_MODIFY) /* modify flags */
+    RtlLockHeap( heap );
+    if ((ptr = unsafe_ptr_from_HLOCAL( handle )))
     {
-        if (unsafe_ptr_from_HLOCAL( handle ) && (flags & LMEM_MOVEABLE))
-        {
-            /* make a fixed block moveable
-             * actually only NT is able to do this. But it's soo simple
-             */
-            if (handle == 0)
-            {
-                WARN_(globalmem)( "null handle\n" );
-                SetLastError( ERROR_NOACCESS );
-            }
-            else
-            {
-                size = RtlSizeHeap( GetProcessHeap(), 0, handle );
-                ret = LocalAlloc( flags, size );
-                ptr = LocalLock( ret );
-                memcpy( ptr, handle, size );
-                LocalUnlock( ret );
-                LocalFree( handle );
-            }
-        }
-        else if ((mem = unsafe_mem_from_HLOCAL( handle )) && (flags & LMEM_DISCARDABLE))
+        if (flags & LMEM_MODIFY) ret = handle;
+        else
         {
-            /* change the flags to make our block "discardable" */
-            mem->flags |= LMEM_DISCARDABLE >> 8;
-            ret = handle;
+            if (!(flags & LMEM_MOVEABLE)) heap_flags |= HEAP_REALLOC_IN_PLACE_ONLY;
+            ret = HeapReAlloc( heap, heap_flags, ptr, size );
+            if (ret) RtlSetUserValueHeap( heap, heap_flags, ret, ret );
+            else SetLastError( ERROR_NOT_ENOUGH_MEMORY );
         }
-        else SetLastError( ERROR_INVALID_PARAMETER );
     }
-    else
+    else if ((mem = unsafe_mem_from_HLOCAL( handle )))
     {
-        if ((ptr = unsafe_ptr_from_HLOCAL( handle )))
-        {
-            /* reallocate fixed memory */
-            if (!(flags & LMEM_MOVEABLE)) heap_flags |= HEAP_REALLOC_IN_PLACE_ONLY;
-            ret = HeapReAlloc( GetProcessHeap(), heap_flags, ptr, size );
-            if (ret) RtlSetUserValueHeap( GetProcessHeap(), heap_flags, ret, ret );
-        }
-        else if ((mem = unsafe_mem_from_HLOCAL( handle )))
+        if (!(flags & LMEM_MODIFY))
         {
-            /* reallocate a moveable block */
-            if (size != 0)
+            if (size)
             {
-                if (size <= INT_MAX)
+                if (!mem->ptr) ptr = HeapAlloc( heap, heap_flags, size );
+                else ptr = HeapReAlloc( heap, heap_flags, mem->ptr, size );
+
+                if (!ptr) SetLastError( ERROR_NOT_ENOUGH_MEMORY );
+                else
                 {
-                    if (mem->ptr)
-                    {
-                        if ((ptr = HeapReAlloc( GetProcessHeap(), heap_flags, mem->ptr, size )))
-                        {
-                            RtlSetUserValueHeap( GetProcessHeap(), heap_flags, ptr, handle );
-                            mem->ptr = ptr;
-                            ret = handle;
-                        }
-                    }
-                    else
-                    {
-                        if ((ptr = HeapAlloc( GetProcessHeap(), heap_flags, size )))
-                        {
-                            RtlSetUserValueHeap( GetProcessHeap(), heap_flags, ptr, handle );
-                            *(HLOCAL *)ptr = handle;
-                            mem->ptr = ptr;
-                            ret = handle;
-                        }
-                    }
+                    RtlSetUserValueHeap( heap, heap_flags, ptr, handle );
+                    mem->flags &= ~MEM_FLAG_DISCARDED;
+                    mem->ptr = ptr;
+                    ret = handle;
                 }
-                else SetLastError( ERROR_OUTOFMEMORY );
             }
             else
             {
-                if (mem->lock == 0)
-                {
-                    if (mem->ptr)
-                    {
-                        HeapFree( GetProcessHeap(), 0, mem->ptr );
-                        mem->ptr = NULL;
-                    }
-                    ret = handle;
-                }
-                else WARN_(globalmem)( "not freeing memory associated with locked handle\n" );
+                HeapFree( heap, heap_flags, mem->ptr );
+                mem->flags |= MEM_FLAG_DISCARDED;
+                mem->lock = 0;
+                mem->ptr = NULL;
+                ret = handle;
             }
         }
-        else SetLastError( ERROR_INVALID_HANDLE );
+        else if (flags & LMEM_DISCARDABLE)
+        {
+            mem->flags |= MEM_FLAG_DISCARDABLE;
+            ret = handle;
+        }
+        else SetLastError( ERROR_INVALID_PARAMETER );
     }
-    RtlUnlockHeap( GetProcessHeap() );
+    else SetLastError( ERROR_INVALID_HANDLE );
+    RtlUnlockHeap( heap );
+
     return ret;
 }
 
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/222



More information about the wine-devel mailing list