[ros-diffs] [reactos] 04/04: [FREELDR] Add RegCloseKey() and use it where registry keys need to be closed.

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=2b3197783833d322fd392…

commit 2b3197783833d322fd3929db074500b253496502
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Fri Nov 19 03:55:15 2021 +0100
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Fri Nov 19 03:55:21 2021 +0100

    [FREELDR] Add RegCloseKey() and use it where registry keys need to be closed.
    
    Actually, RegCloseKey() is here a dummy macro that just "returns"
    success. Indeed, the internal implementation of RegOpenKey() doesn't
    really allocate internal structures: it just returns a "pointer" to
    already existing allocated data. Therefore nothing needs to be closed/freed
    later on.
---
 boot/freeldr/freeldr/ntldr/registry.c   |  15 +++-
 boot/freeldr/freeldr/ntldr/registry.h   |   7 ++
 boot/freeldr/freeldr/ntldr/winldr.c     |   4 ++
 boot/freeldr/freeldr/ntldr/wlregistry.c | 121 +++++++++++++++++---------------
 4 files changed, 89 insertions(+), 58 deletions(-)

diff --git a/boot/freeldr/freeldr/ntldr/registry.c
b/boot/freeldr/freeldr/ntldr/registry.c
index 70a2f2eaa0e..07438b52423 100644
--- a/boot/freeldr/freeldr/ntldr/registry.c
+++ b/boot/freeldr/freeldr/ntldr/registry.c
@@ -58,6 +58,7 @@ RegImportBinaryHive(
     _In_ ULONG ChunkSize)
 {
     NTSTATUS Status;
+
     TRACE("RegImportBinaryHive(%p, 0x%lx)\n", ChunkBase, ChunkSize);
 
     /* Allocate and initialize the hive */
@@ -77,8 +78,8 @@ RegImportBinaryHive(
                           NULL);
     if (!NT_SUCCESS(Status))
     {
-        CmpFree(CmHive, 0);
         ERR("Corrupted hive %p!\n", ChunkBase);
+        CmpFree(CmHive, 0);
         return FALSE;
     }
 
@@ -101,6 +102,7 @@ RegInitCurrentControlSet(
     ULONG LastKnownGoodSet = 0;
     ULONG DataSize;
     LONG Error;
+
     TRACE("RegInitCurrentControlSet\n");
 
     Error = RegOpenKey(NULL,
@@ -121,6 +123,7 @@ RegInitCurrentControlSet(
     if (Error != ERROR_SUCCESS)
     {
         ERR("RegQueryValue('Default') failed (Error %u)\n",
(int)Error);
+        RegCloseKey(SelectKey);
         return Error;
     }
 
@@ -133,9 +136,12 @@ RegInitCurrentControlSet(
     if (Error != ERROR_SUCCESS)
     {
         ERR("RegQueryValue('LastKnownGood') failed (Error %u)\n",
(int)Error);
+        RegCloseKey(SelectKey);
         return Error;
     }
 
+    RegCloseKey(SelectKey);
+
     CurrentSet = (LastKnownGood) ? LastKnownGoodSet : DefaultSet;
     wcscpy(ControlSetKeyName, L"ControlSet");
     switch(CurrentSet)
@@ -169,6 +175,9 @@ RegInitCurrentControlSet(
     Error = RegOpenKey(SystemKey,
                        ControlSetKeyName,
                        &CurrentControlSetKey);
+
+    RegCloseKey(SystemKey);
+
     if (Error != ERROR_SUCCESS)
     {
         ERR("RegOpenKey(CurrentControlSetKey) failed (Error %lu)\n", Error);
@@ -283,6 +292,8 @@ RegEnumKey(
 
     if (SubKey != NULL)
         *SubKey = (HKEY)SubKeyNode;
+    // else
+        // RegCloseKey((HKEY)SubKeyNode);
 
     TRACE("RegEnumKey done -> %u, '%.*S'\n", *NameSize, *NameSize,
Name);
     return ERROR_SUCCESS;
@@ -299,6 +310,7 @@ RegOpenKey(
     PHHIVE Hive = &CmHive->Hive;
     PCM_KEY_NODE KeyNode;
     HCELL_INDEX CellIndex;
+
     TRACE("RegOpenKey(%p, '%S', %p)\n", ParentKey, KeyName, Key);
 
     /* Initialize the remaining path name */
@@ -314,6 +326,7 @@ RegOpenKey(
         UNICODE_STRING RegistryPath = RTL_CONSTANT_STRING(L"Registry");
         UNICODE_STRING MachinePath = RTL_CONSTANT_STRING(L"MACHINE");
         UNICODE_STRING SystemPath = RTL_CONSTANT_STRING(L"SYSTEM");
+
         TRACE("RegOpenKey: absolute path\n");
 
         if ((RemainingPath.Length < sizeof(WCHAR)) ||
diff --git a/boot/freeldr/freeldr/ntldr/registry.h
b/boot/freeldr/freeldr/ntldr/registry.h
index b59cce682f4..8368749339d 100644
--- a/boot/freeldr/freeldr/ntldr/registry.h
+++ b/boot/freeldr/freeldr/ntldr/registry.h
@@ -32,6 +32,13 @@ LONG
 RegInitCurrentControlSet(
     _In_ BOOLEAN LastKnownGood);
 
+/*
+ * LONG
+ * RegCloseKey(
+ *     _In_ HKEY hKey);
+ */
+#define RegCloseKey(hKey)   (ERROR_SUCCESS)
+
 LONG
 RegEnumKey(
     _In_ HKEY Key,
diff --git a/boot/freeldr/freeldr/ntldr/winldr.c b/boot/freeldr/freeldr/ntldr/winldr.c
index a4238a4555a..e1b97336281 100644
--- a/boot/freeldr/freeldr/ntldr/winldr.c
+++ b/boot/freeldr/freeldr/ntldr/winldr.c
@@ -445,6 +445,7 @@ WinLdrDetectVersion(VOID)
         /* Key doesn't exist; assume NT 4.0 */
         return _WIN32_WINNT_NT4;
     }
+    RegCloseKey(hKey);
 
     /* We may here want to read the value of ProductVersion */
     return _WIN32_WINNT_WS03;
@@ -739,11 +740,14 @@ WinLdrInitErrataInf(
     if (rc != ERROR_SUCCESS)
     {
         WARN("Could not retrieve the InfName value (Error %u)\n", (int)rc);
+        RegCloseKey(hKey);
         return FALSE;
     }
 
     // TODO: "SystemBiosDate"
 
+    RegCloseKey(hKey);
+
     RtlStringCbPrintfA(ErrataFilePath, sizeof(ErrataFilePath), "%s%s%S",
                        SystemRoot, "inf\\", szFileName);
 
diff --git a/boot/freeldr/freeldr/ntldr/wlregistry.c
b/boot/freeldr/freeldr/ntldr/wlregistry.c
index 7d9d9219183..83bbda92f84 100644
--- a/boot/freeldr/freeldr/ntldr/wlregistry.c
+++ b/boot/freeldr/freeldr/ntldr/wlregistry.c
@@ -54,7 +54,6 @@ WinLdrLoadSystemHive(
     if (Status != ESUCCESS)
     {
         WARN("Error while opening '%s', Status: %u\n", FullHiveName,
Status);
-        UiMessageBox("Opening hive file failed!");
         return FALSE;
     }
 
@@ -62,8 +61,8 @@ WinLdrLoadSystemHive(
     Status = ArcGetFileInformation(FileId, &FileInfo);
     if (Status != ESUCCESS)
     {
+        WARN("Hive file has 0 size!");
         ArcClose(FileId);
-        UiMessageBox("Hive file has 0 size!");
         return FALSE;
     }
     HiveFileSize = FileInfo.EndingAddress.LowPart;
@@ -75,8 +74,8 @@ WinLdrLoadSystemHive(
 
     if (HiveDataPhysical == NULL)
     {
+        WARN("Could not alloc memory for hive!");
         ArcClose(FileId);
-        UiMessageBox("Unable to alloc memory for a hive!");
         return FALSE;
     }
 
@@ -91,9 +90,8 @@ WinLdrLoadSystemHive(
     Status = ArcRead(FileId, HiveDataPhysical, HiveFileSize, &BytesRead);
     if (Status != ESUCCESS)
     {
-        ArcClose(FileId);
         WARN("Error while reading '%s', Status: %u\n", FullHiveName,
Status);
-        UiMessageBox("Unable to read from hive file!");
+        ArcClose(FileId);
         return FALSE;
     }
 
@@ -102,17 +100,17 @@ WinLdrLoadSystemHive(
     if (FsService)
     {
         BOOLEAN Success;
-        TRACE("  Adding filesystem service %S\n", FsService);
+        TRACE("Adding filesystem service %S\n", FsService);
         Success = WinLdrAddDriverToList(&LoaderBlock->BootDriverListHead,
                                        
L"\\Registry\\Machine\\System\\CurrentControlSet\\Services\\",
                                         NULL,
                                         (PWSTR)FsService);
         if (!Success)
-            TRACE(" Failed to add filesystem service\n");
+            TRACE("Failed to add filesystem service\n");
     }
     else
     {
-        TRACE("  No required filesystem service\n");
+        TRACE("No required filesystem service\n");
     }
 
     ArcClose(FileId);
@@ -147,10 +145,11 @@ WinLdrInitSystemHive(
 
     TRACE("WinLdrInitSystemHive: loading hive %s%s\n", SearchPath, HiveName);
     Success = WinLdrLoadSystemHive(LoaderBlock, SearchPath, HiveName);
-
-    /* Fail if failed... */
     if (!Success)
+    {
+        UiMessageBox("Could not load %s hive!", HiveName);
         return FALSE;
+    }
 
     /* Import what was loaded */
     Success = RegImportBinaryHive(VaToPa(LoaderBlock->RegistryBase),
LoaderBlock->RegistryLength);
@@ -220,8 +219,8 @@ WinLdrGetNLSNames(PSTR AnsiName,
 
     /* Open the CodePage key */
     rc = RegOpenKey(NULL,
-       
L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\NLS\\CodePage",
-        &hKey);
+                   
L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\NLS\\CodePage",
+                    &hKey);
     if (rc != ERROR_SUCCESS)
     {
         //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't open
CodePage registry key");
@@ -234,6 +233,7 @@ WinLdrGetNLSNames(PSTR AnsiName,
     if (rc != ERROR_SUCCESS)
     {
         //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't get ACP NLS
setting");
+        RegCloseKey(hKey);
         return FALSE;
     }
 
@@ -242,6 +242,7 @@ WinLdrGetNLSNames(PSTR AnsiName,
     if (rc != ERROR_SUCCESS)
     {
         //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "ACP NLS Setting exists,
but isn't readable");
+        //RegCloseKey(hKey);
         //return FALSE;
         wcscpy(NameBuffer, L"c_1252.nls"); // HACK: ReactOS bug CORE-6105
     }
@@ -253,6 +254,7 @@ WinLdrGetNLSNames(PSTR AnsiName,
     if (rc != ERROR_SUCCESS)
     {
         //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't get OEMCP
NLS setting");
+        RegCloseKey(hKey);
         return FALSE;
     }
 
@@ -261,15 +263,18 @@ WinLdrGetNLSNames(PSTR AnsiName,
     if (rc != ERROR_SUCCESS)
     {
         //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "OEMCP NLS setting
exists, but isn't readable");
+        //RegCloseKey(hKey);
         //return FALSE;
         wcscpy(NameBuffer, L"c_437.nls"); // HACK: ReactOS bug CORE-6105
     }
     sprintf(OemName, "%S", NameBuffer);
 
+    RegCloseKey(hKey);
+
     /* Open the Language key */
     rc = RegOpenKey(NULL,
-       
L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\NLS\\Language",
-        &hKey);
+                   
L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\NLS\\Language",
+                    &hKey);
     if (rc != ERROR_SUCCESS)
     {
         //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't open
Language registry key");
@@ -282,6 +287,7 @@ WinLdrGetNLSNames(PSTR AnsiName,
     if (rc != ERROR_SUCCESS)
     {
         //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Couldn't get
Language Default setting");
+        RegCloseKey(hKey);
         return FALSE;
     }
 
@@ -290,10 +296,12 @@ WinLdrGetNLSNames(PSTR AnsiName,
     if (rc != ERROR_SUCCESS)
     {
         //RtlStringCbCopyA(szErrorOut, sizeof(szErrorOut), "Language Default setting
exists, but isn't readable");
+        RegCloseKey(hKey);
         return FALSE;
     }
     sprintf(LangName, "%S", NameBuffer);
 
+    RegCloseKey(hKey);
     return TRUE;
 }
 
@@ -482,8 +490,8 @@ WinLdrScanRegistry(IN OUT PLIST_ENTRY BootDriverListHead,
                    IN PCSTR SystemRoot)
 {
     LONG rc = 0;
-    HKEY hGroupKey, hOrderKey, hServiceKey, hDriverKey;
-    PWSTR GroupNameBuffer;
+    HKEY hOrderKey, hServiceKey, hGroupKey, hDriverKey;
+    PWSTR GroupNameBuffer = NULL;
     WCHAR ServiceName[256];
     ULONG OrderList[128];
     ULONG BufferSize;
@@ -503,51 +511,53 @@ WinLdrScanRegistry(IN OUT PLIST_ENTRY BootDriverListHead,
 
     BOOLEAN Success;
 
-    /* get 'service group order' key */
+    /* Get 'group order list' key */
     rc = RegOpenKey(NULL,
-       
L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\ServiceGroupOrder",
-        &hGroupKey);
-    if (rc != ERROR_SUCCESS) {
-
-        TRACE_CH(REACTOS, "Failed to open the 'ServiceGroupOrder' key (rc
%d)\n", (int)rc);
+                   
L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\GroupOrderList",
+                    &hOrderKey);
+    if (rc != ERROR_SUCCESS)
+    {
+        TRACE_CH(REACTOS, "Failed to open the 'GroupOrderList' key (rc
%d)\n", (int)rc);
         return;
     }
 
-    /* get 'group order list' key */
+    /* Get 'services' key */
     rc = RegOpenKey(NULL,
-       
L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\GroupOrderList",
-        &hOrderKey);
-    if (rc != ERROR_SUCCESS) {
-
-        TRACE_CH(REACTOS, "Failed to open the 'GroupOrderList' key (rc
%d)\n", (int)rc);
+                   
L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Services",
+                    &hServiceKey);
+    if (rc != ERROR_SUCCESS)
+    {
+        TRACE_CH(REACTOS, "Failed to open the 'Services' key (rc
%d)\n", (int)rc);
+        RegCloseKey(hOrderKey);
         return;
     }
 
-    /* enumerate drivers */
+    /* Get 'service group order' key */
     rc = RegOpenKey(NULL,
-        L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Services",
-        &hServiceKey);
-    if (rc != ERROR_SUCCESS)  {
-
-        TRACE_CH(REACTOS, "Failed to open the 'Services' key (rc
%d)\n", (int)rc);
-        return;
+                   
L"\\Registry\\Machine\\SYSTEM\\CurrentControlSet\\Control\\ServiceGroupOrder",
+                    &hGroupKey);
+    if (rc != ERROR_SUCCESS)
+    {
+        TRACE_CH(REACTOS, "Failed to open the 'ServiceGroupOrder' key (rc
%d)\n", (int)rc);
+        goto Quit;
     }
 
-    /* Get the Name Group */
+    /* Get the Group Order List */
     BufferSize = 4096;
     GroupNameBuffer = FrLdrHeapAlloc(BufferSize, TAG_WLDR_NAME);
     if (!GroupNameBuffer)
     {
         TRACE_CH(REACTOS, "Failed to allocate buffer\n");
-        return;
+        RegCloseKey(hGroupKey);
+        goto Quit;
     }
     rc = RegQueryValue(hGroupKey, L"List", NULL, (PUCHAR)GroupNameBuffer,
&BufferSize);
-    TRACE_CH(REACTOS, "RegQueryValue(): rc %d\n", (int)rc);
+    RegCloseKey(hGroupKey);
+
     if (rc != ERROR_SUCCESS)
     {
         TRACE_CH(REACTOS, "Failed to query the 'List' value (rc %d)\n",
(int)rc);
-        FrLdrHeapFree(GroupNameBuffer, TAG_WLDR_NAME);
-        return;
+        goto Quit;
     }
     TRACE_CH(REACTOS, "BufferSize: %d\n", (int)BufferSize);
     TRACE_CH(REACTOS, "GroupNameBuffer: '%S'\n", GroupNameBuffer);
@@ -563,12 +573,10 @@ WinLdrScanRegistry(IN OUT PLIST_ENTRY BootDriverListHead,
         rc = RegQueryValue(hOrderKey, GroupName, NULL, (PUCHAR)OrderList,
&BufferSize);
         if (rc != ERROR_SUCCESS) OrderList[0] = 0;
 
-        /* enumerate all drivers */
+        /* Enumerate all drivers */
         for (TagIndex = 1; TagIndex <= OrderList[0]; TagIndex++)
         {
-            Index = 0;
-
-            while (TRUE)
+            for (Index = 0; TRUE; Index++)
             {
                 /* Get the Driver's Name */
                 ValueSize = sizeof(ServiceName);
@@ -579,10 +587,7 @@ WinLdrScanRegistry(IN OUT PLIST_ENTRY BootDriverListHead,
                 if (rc == ERROR_NO_MORE_ITEMS)
                     break;
                 if (rc != ERROR_SUCCESS)
-                {
-                    FrLdrHeapFree(GroupNameBuffer, TAG_WLDR_NAME);
-                    return;
-                }
+                    goto Quit;
                 //TRACE_CH(REACTOS, "Service %d: '%S'\n", (int)Index,
ServiceName);
 
                 /* Read the Start Value */
@@ -643,12 +648,11 @@ WinLdrScanRegistry(IN OUT PLIST_ENTRY BootDriverListHead,
                     //    ServiceName, StartValue, TagValue, DriverGroup,
OrderList[TagIndex], GroupName);
                 }
 
-                Index++;
+                RegCloseKey(hDriverKey);
             }
         }
 
-        Index = 0;
-        while (TRUE)
+        for (Index = 0; TRUE; Index++)
         {
             /* Get the Driver's Name */
             ValueSize = sizeof(ServiceName);
@@ -658,10 +662,7 @@ WinLdrScanRegistry(IN OUT PLIST_ENTRY BootDriverListHead,
             if (rc == ERROR_NO_MORE_ITEMS)
                 break;
             if (rc != ERROR_SUCCESS)
-            {
-                FrLdrHeapFree(GroupNameBuffer, TAG_WLDR_NAME);
-                return;
-            }
+                goto Quit;
             TRACE("Service %d: '%S'\n", (int)Index, ServiceName);
 
             /* Read the Start Value */
@@ -722,15 +723,21 @@ WinLdrScanRegistry(IN OUT PLIST_ENTRY BootDriverListHead,
                 //    ServiceName, StartValue, TagValue, DriverGroup, GroupName);
             }
 
-            Index++;
+            RegCloseKey(hDriverKey);
         }
 
         /* Move to the next group name */
         GroupName = GroupName + wcslen(GroupName) + 1;
     }
 
+Quit:
     /* Free allocated memory */
-    FrLdrHeapFree(GroupNameBuffer, TAG_WLDR_NAME);
+    if (GroupNameBuffer)
+        FrLdrHeapFree(GroupNameBuffer, TAG_WLDR_NAME);
+
+    /* Close the registry key handles */
+    RegCloseKey(hServiceKey);
+    RegCloseKey(hOrderKey);
 }
 
 static