[fireball] 51253: [KERNEL32] - Minor cleanup, better flag names (thanks to ProcessHacker team for the good names and values). - Return error in failure path of BasepGetModuleHandleExW. - Optimize G...
Author: fireball Date: Mon Apr 4 19:35:24 2011 New Revision: 51253
URL: http://svn.reactos.org/svn/reactos?rev=51253&view=rev Log: [KERNEL32] - Minor cleanup, better flag names (thanks to ProcessHacker team for the good names and values). - Return error in failure path of BasepGetModuleHandleExW. - Optimize GetModuleHandleExA so that it calls the internal routine directly, without going through GetModuleHandleExW first and thus validating parameters second time.
Modified: trunk/reactos/dll/ntdll/ldr/ldrapi.c trunk/reactos/dll/win32/kernel32/misc/ldr.c trunk/reactos/include/ndk/ldrtypes.h
Modified: trunk/reactos/dll/ntdll/ldr/ldrapi.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/ntdll/ldr/ldrapi.c?rev=... ============================================================================== --- trunk/reactos/dll/ntdll/ldr/ldrapi.c [iso-8859-1] (original) +++ trunk/reactos/dll/ntdll/ldr/ldrapi.c [iso-8859-1] Mon Apr 4 19:35:24 2011 @@ -15,9 +15,6 @@
/* GLOBALS *******************************************************************/
-#define LDR_LOCK_HELD 0x2 -#define LDR_LOCK_FREE 0x1 - LONG LdrpLoaderLockAcquisitonCount;
/* FUNCTIONS *****************************************************************/ @@ -38,7 +35,7 @@ if (Flags & ~1) { /* Flags are invalid, check how to fail */ - if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS) + if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS) { /* The caller wants us to raise status */ RtlRaiseStatus(STATUS_INVALID_PARAMETER_1); @@ -60,7 +57,7 @@ DPRINT1("LdrUnlockLoaderLock() called with an invalid cookie!\n");
/* Invalid cookie, check how to fail */ - if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS) + if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS) { /* The caller wants us to raise status */ RtlRaiseStatus(STATUS_INVALID_PARAMETER_2); @@ -73,7 +70,7 @@ }
/* Ready to release the lock */ - if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS) + if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS) { /* Do a direct leave */ RtlLeaveCriticalSection(&LdrpLoaderLock); @@ -118,11 +115,11 @@ if (Cookie) *Cookie = 0;
/* Validate the flags */ - if (Flags & ~(LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS | + if (Flags & ~(LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS | LDR_LOCK_LOADER_LOCK_FLAG_TRY_ONLY)) { /* Flags are invalid, check how to fail */ - if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS) + if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS) { /* The caller wants us to raise status */ RtlRaiseStatus(STATUS_INVALID_PARAMETER_1); @@ -136,7 +133,7 @@ if (!Cookie) { /* No cookie check how to fail */ - if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS) + if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS) { /* The caller wants us to raise status */ RtlRaiseStatus(STATUS_INVALID_PARAMETER_3); @@ -150,7 +147,7 @@ if ((Flags & LDR_LOCK_LOADER_LOCK_FLAG_TRY_ONLY) && !(Result)) { /* No pointer to return the data to */ - if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS) + if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS) { /* The caller wants us to raise status */ RtlRaiseStatus(STATUS_INVALID_PARAMETER_2); @@ -164,7 +161,7 @@ if (InInit) return STATUS_SUCCESS;
/* Check what locking semantic to use */ - if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS) + if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS) { /* Check if we should enter or simply try */ if (Flags & LDR_LOCK_LOADER_LOCK_FLAG_TRY_ONLY) @@ -173,13 +170,13 @@ if (!RtlTryEnterCriticalSection(&LdrpLoaderLock)) { /* It's locked */ - *Result = LDR_LOCK_HELD; + *Result = LDR_LOCK_LOADER_LOCK_DISPOSITION_LOCK_NOT_ACQUIRED; goto Quickie; } else { /* It worked */ - *Result = LDR_LOCK_FREE; + *Result = LDR_LOCK_LOADER_LOCK_DISPOSITION_LOCK_ACQUIRED; } } else @@ -188,7 +185,7 @@ RtlEnterCriticalSection(&LdrpLoaderLock);
/* See if result was requested */ - if (Result) *Result = LDR_LOCK_FREE; + if (Result) *Result = LDR_LOCK_LOADER_LOCK_DISPOSITION_LOCK_ACQUIRED; }
/* Increase the acquisition count */ @@ -209,13 +206,13 @@ if (!RtlTryEnterCriticalSection(&LdrpLoaderLock)) { /* It's locked */ - *Result = LDR_LOCK_HELD; + *Result = LDR_LOCK_LOADER_LOCK_DISPOSITION_LOCK_NOT_ACQUIRED; _SEH2_YIELD(return STATUS_SUCCESS); } else { /* It worked */ - *Result = LDR_LOCK_FREE; + *Result = LDR_LOCK_LOADER_LOCK_DISPOSITION_LOCK_ACQUIRED; } } else @@ -224,7 +221,7 @@ RtlEnterCriticalSection(&LdrpLoaderLock);
/* See if result was requested */ - if (Result) *Result = LDR_LOCK_FREE; + if (Result) *Result = LDR_LOCK_LOADER_LOCK_DISPOSITION_LOCK_ACQUIRED; }
/* Increase the acquisition count */
Modified: trunk/reactos/dll/win32/kernel32/misc/ldr.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/misc/ldr... ============================================================================== --- trunk/reactos/dll/win32/kernel32/misc/ldr.c [iso-8859-1] (original) +++ trunk/reactos/dll/win32/kernel32/misc/ldr.c [iso-8859-1] Mon Apr 4 19:35:24 2011 @@ -22,7 +22,7 @@ extern BOOLEAN InWindows; extern WaitForInputIdleType lpfnGlobalRegisterWaitForInputIdle;
-#define BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_FAIL 1 +#define BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_ERROR 1 #define BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_SUCCESS 2 #define BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_CONTINUE 3
@@ -47,14 +47,14 @@ ) { BaseSetLastNTError(STATUS_INVALID_PARAMETER_1); - return BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_FAIL; + return BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_ERROR; }
/* Check 2nd parameter */ if (!phModule) { BaseSetLastNTError(STATUS_INVALID_PARAMETER_2); - return BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_FAIL; + return BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_ERROR; }
/* Return what we have according to the module name */ @@ -578,7 +578,7 @@ Peb = NtCurrentPeb ();
/* Acquire a loader lock */ - LdrLockLoaderLock(LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS, NULL, &Cookie); + LdrLockLoaderLock(LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS, NULL, &Cookie);
/* Traverse the module list */ ModuleListHead = &Peb->Ldr->InLoadOrderModuleList; @@ -615,7 +615,7 @@ } _SEH2_END
/* Release the loader lock */ - LdrUnlockLoaderLock(LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS, Cookie); + LdrUnlockLoaderLock(LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS, Cookie);
return Length / sizeof(WCHAR); } @@ -709,7 +709,8 @@ hModule = GetModuleHandleForUnicodeString(&ModuleNameU); if (!hModule) { - // FIXME: Status?! + /* Last error is already set, so just return failure by setting status */ + Status = STATUS_DLL_NOT_FOUND; goto quickie; } } @@ -737,6 +738,10 @@ hModule); }
+ /* Set last error in case of failure */ + if (!NT_SUCCESS(Status)) + SetLastErrorByStatus(Status); + quickie: /* Unlock loader lock if it was acquired */ if (!NoLock) @@ -744,10 +749,6 @@ Status2 = LdrUnlockLoaderLock(0, Cookie); ASSERT(NT_SUCCESS(Status2)); } - - /* Set last error in case of failure */ - if (!NT_SUCCESS(Status)) - SetLastErrorByStatus(Status);
/* Set the module handle to the caller */ if (phModule) *phModule = hModule; @@ -827,7 +828,7 @@ dwValid = BasepGetModuleHandleExParameterValidation(dwFlags, lpwModuleName, phModule);
/* If result is invalid parameter - return failure */ - if (dwValid == BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_FAIL) return FALSE; + if (dwValid == BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_ERROR) return FALSE;
/* If result is 2, there is no need to do anything - return success. */ if (dwValid == BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_SUCCESS) return TRUE; @@ -855,13 +856,14 @@ { PUNICODE_STRING lpModuleNameW; DWORD dwValid; - BOOL Ret; + BOOL Ret = FALSE; + NTSTATUS Status;
/* Validate parameters */ dwValid = BasepGetModuleHandleExParameterValidation(dwFlags, (LPCWSTR)lpModuleName, phModule);
/* If result is invalid parameter - return failure */ - if (dwValid == BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_FAIL) return FALSE; + if (dwValid == BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_ERROR) return FALSE;
/* If result is 2, there is no need to do anything - return success. */ if (dwValid == BASEP_GET_MODULE_HANDLE_EX_PARAMETER_VALIDATION_SUCCESS) return TRUE; @@ -869,10 +871,11 @@ /* Check if we don't need to convert the name */ if (dwFlags & GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS) { - /* Call the W version of the API without conversion */ - Ret = GetModuleHandleExW(dwFlags, - (LPCWSTR)lpModuleName, - phModule); + /* Call the extended version of the API without conversion */ + Status = BasepGetModuleHandleExW(FALSE, + dwFlags, + (LPCWSTR)lpModuleName, + phModule); } else { @@ -882,11 +885,16 @@ /* Return FALSE if conversion failed */ if (!lpModuleNameW) return FALSE;
- /* Call the W version of the API */ - Ret = GetModuleHandleExW(dwFlags, - lpModuleNameW->Buffer, - phModule); - } + /* Call the extended version of the API */ + Status = BasepGetModuleHandleExW(FALSE, + dwFlags, + lpModuleNameW->Buffer, + phModule); + } + + /* If result was successful - return true */ + if (NT_SUCCESS(Status)) + Ret = TRUE;
/* Return result */ return Ret;
Modified: trunk/reactos/include/ndk/ldrtypes.h URL: http://svn.reactos.org/svn/reactos/trunk/reactos/include/ndk/ldrtypes.h?rev=... ============================================================================== --- trunk/reactos/include/ndk/ldrtypes.h [iso-8859-1] (original) +++ trunk/reactos/include/ndk/ldrtypes.h [iso-8859-1] Mon Apr 4 19:35:24 2011 @@ -71,8 +71,12 @@ // // LdrLockLoaderLock Flags // -#define LDR_LOCK_LOADER_LOCK_FLAG_RAISE_STATUS 0x00000001 -#define LDR_LOCK_LOADER_LOCK_FLAG_TRY_ONLY 0x00000002 +#define LDR_LOCK_LOADER_LOCK_FLAG_RAISE_ON_ERRORS 0x00000001 +#define LDR_LOCK_LOADER_LOCK_FLAG_TRY_ONLY 0x00000002 + +#define LDR_LOCK_LOADER_LOCK_DISPOSITION_INVALID 0 +#define LDR_LOCK_LOADER_LOCK_DISPOSITION_LOCK_ACQUIRED 1 +#define LDR_LOCK_LOADER_LOCK_DISPOSITION_LOCK_NOT_ACQUIRED 2
// // FIXME: THIS SHOULD *NOT* BE USED!
-
fireball＠svn.reactos.org