From 7fcd953546f81e00f680c0e8d5634f0549908c2c Mon Sep 17 00:00:00 2001 From: Aleksey Bragin Date: Mon, 4 Apr 2011 19:35:24 +0000 Subject: [PATCH] [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. svn path=/trunk/; revision=51253 --- reactos/dll/ntdll/ldr/ldrapi.c | 31 ++++++++--------- reactos/dll/win32/kernel32/misc/ldr.c | 50 ++++++++++++++++----------- reactos/include/ndk/ldrtypes.h | 8 +++-- 3 files changed, 49 insertions(+), 40 deletions(-) diff --git a/reactos/dll/ntdll/ldr/ldrapi.c b/reactos/dll/ntdll/ldr/ldrapi.c index 9b697ca9a0e..e08f4ca5ec8 100644 --- a/reactos/dll/ntdll/ldr/ldrapi.c +++ b/reactos/dll/ntdll/ldr/ldrapi.c @@ -15,9 +15,6 @@ /* GLOBALS *******************************************************************/ -#define LDR_LOCK_HELD 0x2 -#define LDR_LOCK_FREE 0x1 - LONG LdrpLoaderLockAcquisitonCount; /* FUNCTIONS *****************************************************************/ @@ -38,7 +35,7 @@ LdrUnlockLoaderLock(IN ULONG Flags, 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 @@ LdrUnlockLoaderLock(IN ULONG Flags, 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 @@ LdrUnlockLoaderLock(IN ULONG Flags, } /* 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 @@ LdrLockLoaderLock(IN ULONG Flags, 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 @@ LdrLockLoaderLock(IN ULONG Flags, 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 @@ LdrLockLoaderLock(IN ULONG Flags, 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 @@ LdrLockLoaderLock(IN ULONG Flags, 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 @@ LdrLockLoaderLock(IN ULONG Flags, 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 @@ LdrLockLoaderLock(IN ULONG Flags, 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 @@ LdrLockLoaderLock(IN ULONG Flags, 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 @@ LdrLockLoaderLock(IN ULONG Flags, 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 */ diff --git a/reactos/dll/win32/kernel32/misc/ldr.c b/reactos/dll/win32/kernel32/misc/ldr.c index 8dd3d9994dc..cbf00e56896 100644 --- a/reactos/dll/win32/kernel32/misc/ldr.c +++ b/reactos/dll/win32/kernel32/misc/ldr.c @@ -22,7 +22,7 @@ typedef struct tagLOADPARMS32 { 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 @@ BasepGetModuleHandleExParameterValidation(DWORD dwFlags, ) { 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 @@ GetModuleFileNameW(HINSTANCE hModule, 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 @@ GetModuleFileNameW(HINSTANCE hModule, } _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 @@ BasepGetModuleHandleExW(BOOLEAN NoLock, DWORD dwPublicFlags, LPCWSTR lpwModuleNa 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 @@ BasepGetModuleHandleExW(BOOLEAN NoLock, DWORD dwPublicFlags, LPCWSTR lpwModuleNa hModule); } + /* Set last error in case of failure */ + if (!NT_SUCCESS(Status)) + SetLastErrorByStatus(Status); + quickie: /* Unlock loader lock if it was acquired */ if (!NoLock) @@ -745,10 +750,6 @@ quickie: 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 @@ GetModuleHandleExW(IN DWORD dwFlags, 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 @@ GetModuleHandleExA(IN DWORD dwFlags, { 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 @@ GetModuleHandleExA(IN DWORD dwFlags, /* 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,12 +885,17 @@ GetModuleHandleExA(IN DWORD dwFlags, /* 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; } diff --git a/reactos/include/ndk/ldrtypes.h b/reactos/include/ndk/ldrtypes.h index 4586f8325e5..8887a040b07 100644 --- a/reactos/include/ndk/ldrtypes.h +++ b/reactos/include/ndk/ldrtypes.h @@ -71,8 +71,12 @@ Author: // // 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!