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=51253&r1=51252&r2=51253&view=diff
==============================================================================
--- 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.c?rev=51253&r1=51252&r2=51253&view=diff
==============================================================================
--- 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=51253&r1=51252&r2=51253&view=diff
==============================================================================
--- 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!


Reply via email to