[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/cursori... ============================================================================== --- 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/windows... ============================================================================== --- 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);
-
jgardou＠svn.reactos.org