[ros-diffs] [jgardou] 64938: [WIN32K/USER32] - Simplify an overcomplicated way to get buffer size from win32k - Do not spaghettize between string length and buffer size. - Properly use GetModuleFileName.

Author: jgardou
Date: Thu Oct 23 19:41:42 2014
New Revision: 64938

URL: http://svn.reactos.org/svn/reactos?rev=64938&view=rev
Log:
[WIN32K/USER32]
 - Simplify an overcomplicated way to get buffer size from win32k
 - Do not spaghettize between string length and buffer size.
 - Properly use GetModuleFileName.

Modified:
    trunk/reactos/win32ss/user/ntuser/cursoricon_new.c
    trunk/reactos/win32ss/user/user32/windows/cursoricon_new.c

Modified: trunk/reactos/win32ss/user/ntuser/cursoricon_new.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/ntuser/cursor…
==============================================================================
--- trunk/reactos/win32ss/user/ntuser/cursoricon_new.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/ntuser/cursoricon_new.c	[iso-8859-1] Thu Oct 23 19:41:42
2014
@@ -187,7 +187,8 @@
 
     /* We just mark the handle as being destroyed.
      * Deleting all the stuff will be deferred to the actual struct free. */
-    return UserDeleteObject(CurIcon->head.h, TYPE_CURSOR);
+    UserDeleteObject(CurIcon->head.h, TYPE_CURSOR);
+    return TRUE;
 }
 
 void
@@ -357,16 +358,18 @@
         /* Get the module name from the atom table */
         _SEH2_TRY
         {
-            if (BufLen > (lpModule->MaximumLength * sizeof(WCHAR)))
+            BufLen += sizeof(WCHAR);
+            if (BufLen > (lpModule->MaximumLength))
             {
                 lpModule->Length = 0;
+                lpModule->MaximumLength = BufLen;
             }
             else
             {
                 ProbeForWrite(lpModule->Buffer, lpModule->MaximumLength, 1);
-                BufLen = lpModule->MaximumLength * sizeof(WCHAR);
+                BufLen = lpModule->MaximumLength;
                 RtlQueryAtomInAtomTable(gAtomTable, CurIcon->atomModName, NULL, NULL,
lpModule->Buffer, &BufLen);
-                lpModule->Length = BufLen/sizeof(WCHAR);
+                lpModule->Length = BufLen;
             }
         }
         _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)
@@ -395,15 +398,18 @@
             {
                 lpResName->Buffer = CurIcon->strName.Buffer;
                 lpResName->Length = 0;
+                lpResName->MaximumLength = 0;
             }
-            else if (lpResName->MaximumLength < CurIcon->strName.Length)
+            else if (lpResName->MaximumLength < CurIcon->strName.MaximumLength)
             {
                 lpResName->Length = 0;
+                lpResName->MaximumLength = CurIcon->strName.MaximumLength;
             }
             else
             {
-                ProbeForWrite(lpResName->Buffer, lpResName->MaximumLength *
sizeof(WCHAR), 1);
-                RtlCopyMemory(lpResName->Buffer, CurIcon->strName.Buffer,
lpResName->Length);
+                ProbeForWrite(lpResName->Buffer, lpResName->MaximumLength, 1);
+                RtlCopyMemory(lpResName->Buffer, CurIcon->strName.Buffer,
CurIcon->strName.Length);
+                lpResName->Length = CurIcon->strName.Length;
             }
         }
         _SEH2_EXCEPT(EXCEPTION_EXECUTE_HANDLER)

Modified: trunk/reactos/win32ss/user/user32/windows/cursoricon_new.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/win32ss/user/user32/window…
==============================================================================
--- trunk/reactos/win32ss/user/user32/windows/cursoricon_new.c	[iso-8859-1] (original)
+++ trunk/reactos/win32ss/user/user32/windows/cursoricon_new.c	[iso-8859-1] Thu Oct 23
19:41:42 2014
@@ -1342,27 +1342,37 @@
         }
         else
             RtlInitUnicodeString(&ustrRsrc, lpszName);
-        
-        /* Prepare the module name string */
-        ustrModule.Buffer = HeapAlloc(GetProcessHeap(), 0, size*sizeof(WCHAR));
-        /* Get it */
-        do
-        {
-            DWORD ret = GetModuleFileNameW(hinst, ustrModule.Buffer, size);
+
+        /* Get the module name string */
+        while (TRUE)
+        {
+            DWORD ret;
+            ustrModule.Buffer = HeapAlloc(GetProcessHeap(), 0, size*sizeof(WCHAR));
+            if (!ustrModule.Buffer)
+            {
+                SetLastError(ERROR_NOT_ENOUGH_MEMORY);
+                return NULL;
+            }
+            ret = GetModuleFileNameW(hinst, ustrModule.Buffer, size);
             if(ret == 0)
             {
                 HeapFree(GetProcessHeap(), 0, ustrModule.Buffer);
                 return NULL;
             }
-            if(ret < size)
+
+            /* This API is completely broken... */
+            if (ret == size)
             {
-                ustrModule.Length = ret*sizeof(WCHAR);
-                ustrModule.MaximumLength = size*sizeof(WCHAR);
-                break;
+                HeapFree(GetProcessHeap(), 0, ustrModule.Buffer);
+                size *= 2;
+                continue;
             }
-            size *= 2;
-            ustrModule.Buffer = HeapReAlloc(GetProcessHeap(), 0, ustrModule.Buffer,
size*sizeof(WCHAR));
-        } while(TRUE);
+
+            ustrModule.Buffer[ret] = UNICODE_NULL;
+            ustrModule.Length = ret * sizeof(WCHAR);
+            ustrModule.MaximumLength = size * sizeof(WCHAR);
+            break;
+        }
         
         /* Ask win32k */
         param.bIcon = bIcon;
@@ -1691,75 +1701,51 @@
         /* Get the icon module/resource names */
         UNICODE_STRING ustrModule;
         UNICODE_STRING ustrRsrc;
-        PVOID pvBuf;
         HMODULE hModule;
         
-        ustrModule.MaximumLength = MAX_PATH * sizeof(WCHAR);
-        ustrRsrc.MaximumLength = 256;
-        
+        ustrModule.MaximumLength = 0;
+        ustrRsrc.MaximumLength = 0;
+        
+        /* Get the buffer size */
+        if (!NtUserGetIconInfo(hicon, NULL, &ustrModule, &ustrRsrc, NULL,
FALSE))
+        {
+            return NULL;
+        }
+
         ustrModule.Buffer = HeapAlloc(GetProcessHeap(), 0, ustrModule.MaximumLength);
         if (!ustrModule.Buffer)
         {
             SetLastError(ERROR_NOT_ENOUGH_MEMORY);
             return NULL;
         }
-        /* Keep track of the buffer for the resource, NtUserGetIconInfo might overwrite
it */
-        pvBuf = HeapAlloc(GetProcessHeap(), 0, ustrRsrc.MaximumLength);
-        if (!pvBuf)
-        {
-            HeapFree(GetProcessHeap(), 0, ustrModule.Buffer);
-            SetLastError(ERROR_NOT_ENOUGH_MEMORY);
-            return NULL;
-        }
-        ustrRsrc.Buffer = pvBuf;
-        
-        do
-        {
-            if (!NtUserGetIconInfo(hicon, NULL, &ustrModule, &ustrRsrc, NULL,
FALSE))
+        
+        if (ustrRsrc.MaximumLength)
+        {
+            ustrRsrc.Buffer = HeapAlloc(GetProcessHeap(), 0, ustrRsrc.MaximumLength);
+            if (!ustrRsrc.Buffer)
             {
                 HeapFree(GetProcessHeap(), 0, ustrModule.Buffer);
-                HeapFree(GetProcessHeap(), 0, pvBuf);
+                SetLastError(ERROR_NOT_ENOUGH_MEMORY);
                 return NULL;
             }
-            
-            if (ustrModule.Length && (ustrRsrc.Length ||
IS_INTRESOURCE(ustrRsrc.Buffer)))
-            {
-                /* Buffers were big enough */
-                break;
-            }
-            
-            /* Find which buffer were too small */
-            if (!ustrModule.Length)
-            {
-                PWSTR newBuffer;
-                ustrModule.MaximumLength *= 2;
-                newBuffer = HeapReAlloc(GetProcessHeap(), 0, ustrModule.Buffer,
ustrModule.MaximumLength);
-                if(!ustrModule.Buffer)
-                {
-                    SetLastError(ERROR_NOT_ENOUGH_MEMORY);
-                    goto leave;
-                }
-                ustrModule.Buffer = newBuffer;
-            }
-            
-            if (!ustrRsrc.Length)
-            {
-                ustrRsrc.MaximumLength *= 2;
-                pvBuf = HeapReAlloc(GetProcessHeap(), 0, ustrRsrc.Buffer,
ustrRsrc.MaximumLength);
-                if (!pvBuf)
-                {
-                    SetLastError(ERROR_NOT_ENOUGH_MEMORY);
-                    goto leave;
-                }
-                ustrRsrc.Buffer = pvBuf;
-            }
-        } while(TRUE);
+        }
+
+        if (!NtUserGetIconInfo(hicon, NULL, &ustrModule, &ustrRsrc, NULL,
FALSE))
+        {
+            HeapFree(GetProcessHeap(), 0, ustrModule.Buffer);
+            if (!IS_INTRESOURCE(ustrRsrc.Buffer))
+                HeapFree(GetProcessHeap(), 0, ustrRsrc.Buffer);
+            return NULL;
+        }
         
         /* NULL-terminate our strings */
-        ustrModule.Buffer[ustrModule.Length/sizeof(WCHAR)] = 0;
+        ustrModule.Buffer[ustrModule.Length/sizeof(WCHAR)] = UNICODE_NULL;
         if (!IS_INTRESOURCE(ustrRsrc.Buffer))
-            ustrRsrc.Buffer[ustrRsrc.Length/sizeof(WCHAR)] = 0;
-        
+            ustrRsrc.Buffer[ustrRsrc.Length/sizeof(WCHAR)] = UNICODE_NULL;
+        
+        TRACE("Got module %S, resource %p (%S).\n", ustrModule.Buffer,
+            ustrRsrc.Buffer, IS_INTRESOURCE(ustrRsrc.Buffer) ? L"" :
ustrRsrc.Buffer);
+
         /* Get the module handle */
         if (!GetModuleHandleExW(0, ustrModule.Buffer, &hModule))
         {
@@ -1783,7 +1769,8 @@
         /* If we're here, that means that the passed icon is shared. Don't
destroy it, even if LR_COPYDELETEORG is specified */
     leave:
         HeapFree(GetProcessHeap(), 0, ustrModule.Buffer);
-        HeapFree(GetProcessHeap(), 0, pvBuf);
+        if (!IS_INTRESOURCE(ustrRsrc.Buffer))
+            HeapFree(GetProcessHeap(), 0, ustrRsrc.Buffer);
         
         TRACE("Returning 0x%08x.\n", ret);