[SETUPLIB] Properly cleanup the temporary registry keys created for setting up the registry.

This includes also to remove the created symlinks.
Symlinks deletion is special, as one has to open first the symlink
itself (and not its target), then remove the "SymbolicLinkTarget" value
before really deleting the key. Of course everything must be done under
proper access rights.

Additional changes:
- Change prototype BOOLEAN CmpLinkKeyToHive(...) to NTSTATUS CreateSymLinkKey(...).
- Silence few DPRINTs.
- Document some FIXMEs that I need to inspect later on ReactOS.
- HKEY --> HANDLE.

base/setup/lib/registry.c

@@ -67,7 +67,7 @@
 #define Architecture L"ppc"
 #endif
 
-/* FUNCTIONS ****************************************************************/
+/* GLOBALS ******************************************************************/
 
 #define REGISTRY_SETUP_MACHINE  L"\\Registry\\Machine\\SYSTEM\\USetup_Machine\\"
 #define REGISTRY_SETUP_USER     L"\\Registry\\Machine\\SYSTEM\\USetup_User\\"
@@ -90,6 +90,8 @@ ROOT_KEY RootKeys[] =
 #endif
 };
 
+/* FUNCTIONS ****************************************************************/
+
 #define IsPredefKey(HKey)       \
     (((ULONG_PTR)(HKey) & 0xF0000000) == 0x80000000)
 
@@ -789,7 +791,9 @@ RegInitializeRegistry(
                          &ObjectAttributes,
                          0,
                          NULL,
-                         REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE, // FIXME!
+                    // FIXME: Using REG_OPTION_VOLATILE works OK on Windows,
+                    // but I need to check whether it works OK on ReactOS too.
+                         REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE,
                          &Disposition);
     if (!NT_SUCCESS(Status))
     {
@@ -811,7 +815,9 @@ RegInitializeRegistry(
                          &ObjectAttributes,
                          0,
                          NULL,
-                         REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE, // FIXME!
+                    // FIXME: Using REG_OPTION_VOLATILE works OK on Windows,
+                    // but I need to check whether it works OK on ReactOS too.
+                         REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE,
                          &Disposition);
     if (!NT_SUCCESS(Status))
     {
@@ -838,15 +844,18 @@ RegInitializeRegistry(
                                      /* SystemSecurity, sizeof(SystemSecurity) */);
             if (!NT_SUCCESS(Status))
             {
-                DPRINT1("ConnectRegistry(%S) failed, Status 0x%08lx\n", RegistryHives[i].HiveName, Status);
+                DPRINT1("ConnectRegistry(%S) failed, Status 0x%08lx\n",
+                        RegistryHives[i].HiveName, Status);
             }
 
             /* Create the registry symlink to this key */
-            if (!CmpLinkKeyToHive(RootKeys[GetPredefKeyIndex(RegistryHives[i].PredefKeyHandle)].Handle,
+            Status = CreateSymLinkKey(RootKeys[GetPredefKeyIndex(RegistryHives[i].PredefKeyHandle)].Handle,
                                       RegistryHives[i].RegSymLink,
-                                  RegistryHives[i].HiveRegistryPath))
+                                      RegistryHives[i].HiveRegistryPath);
+            if (!NT_SUCCESS(Status))
             {
-                DPRINT1("CmpLinkKeyToHive(%S) failed!\n", RegistryHives[i].HiveName);
+                DPRINT1("CreateSymLinkKey(%S) failed, Status 0x%08lx\n",
+                        RegistryHives[i].RegSymLink, Status);
             }
         }
         else
@@ -865,7 +874,9 @@ RegInitializeRegistry(
                                  &ObjectAttributes,
                                  0,
                                  NULL,
-                                 REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE, // FIXME!
+                            // FIXME: Using REG_OPTION_VOLATILE works OK on Windows,
+                            // but I need to check whether it works OK on ReactOS too.
+                                 REG_OPTION_NON_VOLATILE, // REG_OPTION_VOLATILE,
                                  &Disposition);
             if (!NT_SUCCESS(Status))
             {
@@ -935,7 +946,7 @@ RegInitializeRegistry(
     }
     else
     {
-        DPRINT1("NtCreateKey() succeeded to %s the %wZ key (Status %lx)\n",
+        DPRINT("NtCreateKey() succeeded to %s the %wZ key (Status %lx)\n",
                Disposition == REG_CREATED_NEW_KEY ? "create" : /* REG_OPENED_EXISTING_KEY */ "open",
                &KeyName, Status);
     }
@@ -967,18 +978,19 @@ RegInitializeRegistry(
     }
     else
     {
-        DPRINT1("NtCreateKey() succeeded to %s the ControlSet001 key (Status %lx)\n",
+        DPRINT("NtCreateKey() succeeded to %s the ControlSet001 key (Status %lx)\n",
                Disposition == REG_CREATED_NEW_KEY ? "create" : /* REG_OPENED_EXISTING_KEY */ "open",
                Status);
     }
     NtClose(KeyHandle);
 
     /* Create the 'HKLM\SYSTEM\CurrentControlSet' symlink */
-    if (!CmpLinkKeyToHive(RootKeys[GetPredefKeyIndex(HKEY_LOCAL_MACHINE)].Handle,
+    Status = CreateSymLinkKey(RootKeys[GetPredefKeyIndex(HKEY_LOCAL_MACHINE)].Handle,
                               L"SYSTEM\\CurrentControlSet",
-                          REGISTRY_SETUP_MACHINE L"SYSTEM\\ControlSet001"))
+                              REGISTRY_SETUP_MACHINE L"SYSTEM\\ControlSet001");
+    if (!NT_SUCCESS(Status))
     {
-        DPRINT1("CmpLinkKeyToHive(CurrentControlSet) failed!\n");
+        DPRINT1("CreateSymLinkKey(CurrentControlSet) failed, Status 0x%08lx\n", Status);
     }
 
 
@@ -1026,15 +1038,37 @@ RegCleanupRegistry(
     }
 
     /*
-     * Note that we don't need to explicitly remove the symlinks we have created
+     * To keep the running system clean we need first to remove the symlinks
-     * since they are created volatile, inside registry keys that will be however
+     * we have created and then unmounting the hives. Finally we delete the
-     * removed explictly in the following.
+     * master registry keys.
      */
 
     for (i = 0; i < ARRAYSIZE(RegistryHives); ++i)
     {
-        if (RegistryHives[i].State != Create && RegistryHives[i].State != Repair)
+        if (RegistryHives[i].State == Create || RegistryHives[i].State == Repair)
         {
+            /* Delete the registry symlink to this key */
+            Status = DeleteSymLinkKey(RootKeys[GetPredefKeyIndex(RegistryHives[i].PredefKeyHandle)].Handle,
+                                      RegistryHives[i].RegSymLink);
+            if (!NT_SUCCESS(Status))
+            {
+                DPRINT1("DeleteSymLinkKey(%S) failed, Status 0x%08lx\n",
+                        RegistryHives[i].RegSymLink, Status);
+            }
+
+            /* Unmount the hive */
+            Status = DisconnectRegistry(NULL,
+                                        RegistryHives[i].HiveRegistryPath,
+                                        1 /* REG_FORCE_UNLOAD */);
+            DPRINT1("Unmounting '%S' %s\n", RegistryHives[i].HiveRegistryPath, NT_SUCCESS(Status) ? "succeeded" : "failed");
+
+            /* Switch the hive state to 'Update' */
+            RegistryHives[i].State = Update;
+        }
+        else
+        {
+            /* Delete the *DUMMY* volatile hives created for the update procedure */
+
             RtlInitUnicodeString(&KeyName, RegistryHives[i].RegSymLink);
             InitializeObjectAttributes(&ObjectAttributes,
                                        &KeyName,
@@ -1054,16 +1088,6 @@ RegCleanupRegistry(
             NtDeleteKey(KeyHandle);
             NtClose(KeyHandle);
         }
-        else
-        {
-            Status = DisconnectRegistry(NULL,
-                                        RegistryHives[i].HiveRegistryPath,
-                                        1 /* REG_FORCE_UNLOAD */);
-            DPRINT1("Unmounting '%S' %s\n", RegistryHives[i].HiveRegistryPath, NT_SUCCESS(Status) ? "succeeded" : "failed");
-
-            /* Switch the hive state to 'Update' */
-            RegistryHives[i].State = Update;
-        }
     }
 
     /*

base/setup/lib/utils/regutil.c

@@ -16,6 +16,11 @@
 #define NDEBUG
 #include <debug.h>
 
+/* GLOBALS ******************************************************************/
+
+static UNICODE_STRING SymbolicLinkValueName =
+    RTL_CONSTANT_STRING(L"SymbolicLinkValue");
+
 /* FUNCTIONS ****************************************************************/
 
 /*
@@ -80,7 +85,7 @@ CreateNestedKey(PHANDLE KeyHandle,
                              &LocalObjectAttributes,
                              0,
                              NULL,
-                             REG_OPTION_NON_VOLATILE,
+                             REG_OPTION_NON_VOLATILE, // FIXME ?
                              &Disposition);
         DPRINT("NtCreateKey(%wZ) called (Status %lx)\n", &LocalKeyName, Status);
         if (!NT_SUCCESS(Status) && Status != STATUS_OBJECT_NAME_NOT_FOUND)
@@ -244,19 +249,17 @@ CreateRegistryFile(
     return Status;
 }
 
-BOOLEAN
+/* Adapted from ntoskrnl/config/cmsysini.c:CmpLinkKeyToHive() */
-CmpLinkKeyToHive(
+NTSTATUS
-    IN HANDLE RootLinkKeyHandle OPTIONAL,
+CreateSymLinkKey(
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR LinkKeyName,
     IN PCWSTR TargetKeyName)
 {
-    static UNICODE_STRING CmSymbolicLinkValueName =
-        RTL_CONSTANT_STRING(L"SymbolicLinkValue");
-
     NTSTATUS Status;
     OBJECT_ATTRIBUTES ObjectAttributes;
     UNICODE_STRING KeyName;
-    HANDLE TargetKeyHandle;
+    HANDLE LinkKeyHandle;
     ULONG Disposition;
 
     /* Initialize the object attributes */
@@ -264,11 +267,11 @@ CmpLinkKeyToHive(
     InitializeObjectAttributes(&ObjectAttributes,
                                &KeyName,
                                OBJ_CASE_INSENSITIVE,
-                               RootLinkKeyHandle,
+                               RootKey,
                                NULL);
 
     /* Create the link key */
-    Status = NtCreateKey(&TargetKeyHandle,
+    Status = NtCreateKey(&LinkKeyHandle,
                          KEY_SET_VALUE | KEY_CREATE_LINK,
                          &ObjectAttributes,
                          0,
@@ -277,39 +280,108 @@ CmpLinkKeyToHive(
                          &Disposition);
     if (!NT_SUCCESS(Status))
     {
-        DPRINT1("CmpLinkKeyToHive: couldn't create %S, Status = 0x%08lx\n",
+        DPRINT1("CreateSymLinkKey: couldn't create '%S', Status = 0x%08lx\n",
                 LinkKeyName, Status);
-        return FALSE;
+        return Status;
     }
 
     /* Check if the new key was actually created */
     if (Disposition != REG_CREATED_NEW_KEY)
     {
-        DPRINT1("CmpLinkKeyToHive: %S already exists!\n", LinkKeyName);
+        DPRINT1("CreateSymLinkKey: %S already exists!\n", LinkKeyName);
-        NtClose(TargetKeyHandle);
+        NtClose(LinkKeyHandle);
-        return FALSE;
+        return STATUS_OBJECT_NAME_EXISTS; // STATUS_OBJECT_NAME_COLLISION;
     }
 
     /* Set the target key name as link target */
     RtlInitUnicodeString(&KeyName, TargetKeyName);
-    Status = NtSetValueKey(TargetKeyHandle,
+    Status = NtSetValueKey(LinkKeyHandle,
-                           &CmSymbolicLinkValueName,
+                           &SymbolicLinkValueName,
                            0,
                            REG_LINK,
                            KeyName.Buffer,
                            KeyName.Length);
 
     /* Close the link key handle */
-    NtClose(TargetKeyHandle);
+    NtClose(LinkKeyHandle);
 
     if (!NT_SUCCESS(Status))
     {
-        DPRINT1("CmpLinkKeyToHive: couldn't create symbolic link for %S, Status = 0x%08lx\n",
+        DPRINT1("CreateSymLinkKey: couldn't create symbolic link '%S' for '%S', Status = 0x%08lx\n",
-                TargetKeyName, Status);
+                LinkKeyName, TargetKeyName, Status);
-        return FALSE;
     }
 
-    return TRUE;
+    return Status;
+}
+
+NTSTATUS
+DeleteSymLinkKey(
+    IN HANDLE RootKey OPTIONAL,
+    IN PCWSTR LinkKeyName)
+{
+    NTSTATUS Status;
+    OBJECT_ATTRIBUTES ObjectAttributes;
+    UNICODE_STRING KeyName;
+    HANDLE LinkKeyHandle;
+    // ULONG Disposition;
+
+    /* Initialize the object attributes */
+    RtlInitUnicodeString(&KeyName, LinkKeyName);
+    InitializeObjectAttributes(&ObjectAttributes,
+                               &KeyName,
+            /* Open the symlink key itself if it exists, and not its target */
+                               OBJ_CASE_INSENSITIVE | OBJ_OPENIF | OBJ_OPENLINK,
+                               RootKey,
+                               NULL);
+
+    /*
+     * Note: We could use here NtOpenKey() but it does not allow to pass
+     * opening options. NtOpenKeyEx() could do it but is Windows 7+.
+     * So we use the good old NtCreateKey() that can open the key.
+     */
+#if 0
+    Status = NtCreateKey(&LinkKeyHandle,
+                         DELETE | KEY_SET_VALUE | KEY_CREATE_LINK,
+                         &ObjectAttributes,
+                         0,
+                         NULL,
+                         /*REG_OPTION_VOLATILE |*/ REG_OPTION_OPEN_LINK,
+                         &Disposition);
+#else
+    Status = NtOpenKey(&LinkKeyHandle,
+                       DELETE | KEY_SET_VALUE | KEY_CREATE_LINK,
+                       &ObjectAttributes);
+#endif
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("NtOpenKey(%wZ) failed (Status %lx)\n", &KeyName, Status);
+        return Status;
+    }
+
+    /*
+     * Delete the special "SymbolicLinkValue" value.
+     * This is technically not needed since we are going to remove
+     * the key anyways, but it is good practice to do it.
+     */
+    Status = NtDeleteValueKey(LinkKeyHandle, &SymbolicLinkValueName);
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("NtDeleteValueKey(%wZ) failed (Status %lx)\n", &KeyName, Status);
+        NtClose(LinkKeyHandle);
+        return Status;
+    }
+
+    /* Finally delete the key itself and close the link key handle */
+    Status = NtDeleteKey(LinkKeyHandle);
+    NtClose(LinkKeyHandle);
+
+    if (!NT_SUCCESS(Status))
+    {
+        DPRINT1("DeleteSymLinkKey: couldn't delete symbolic link '%S', Status = 0x%08lx\n",
+                LinkKeyName, Status);
+    }
+
+    return Status;
 }
 
 /*
@@ -317,7 +389,7 @@ CmpLinkKeyToHive(
  */
 NTSTATUS
 ConnectRegistry(
-    IN HKEY RootKey OPTIONAL,
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR RegMountPoint,
     // IN HANDLE RootDirectory OPTIONAL,
     IN PUNICODE_STRING NtSystemRoot,
@@ -358,7 +430,7 @@ ConnectRegistry(
  */
 NTSTATUS
 DisconnectRegistry(
-    IN HKEY RootKey OPTIONAL,
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR RegMountPoint,
     IN ULONG Flags)
 {
@@ -381,7 +453,7 @@ DisconnectRegistry(
  */
 NTSTATUS
 VerifyRegistryHive(
-    // IN HKEY RootKey OPTIONAL,
+    // IN HANDLE RootKey OPTIONAL,
     // // IN HANDLE RootDirectory OPTIONAL,
     IN PUNICODE_STRING NtSystemRoot,
     IN PCWSTR RegistryKey /* ,

base/setup/lib/utils/regutil.h

@@ -35,18 +35,24 @@ CreateRegistryFile(
 */
     );
 
-BOOLEAN
+/* Adapted from ntoskrnl/config/cmsysini.c:CmpLinkKeyToHive() */
-CmpLinkKeyToHive(
+NTSTATUS
-    IN HANDLE RootLinkKeyHandle OPTIONAL,
+CreateSymLinkKey(
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR LinkKeyName,
     IN PCWSTR TargetKeyName);
 
+NTSTATUS
+DeleteSymLinkKey(
+    IN HANDLE RootKey OPTIONAL,
+    IN PCWSTR LinkKeyName);
+
 /*
  * Should be called under SE_RESTORE_PRIVILEGE privilege
  */
 NTSTATUS
 ConnectRegistry(
-    IN HKEY RootKey OPTIONAL,
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR RegMountPoint,
     // IN HANDLE RootDirectory OPTIONAL,
     IN PUNICODE_STRING NtSystemRoot,
@@ -62,7 +68,7 @@ ConnectRegistry(
  */
 NTSTATUS
 DisconnectRegistry(
-    IN HKEY RootKey OPTIONAL,
+    IN HANDLE RootKey OPTIONAL,
     IN PCWSTR RegMountPoint,
     IN ULONG Flags);
 
@@ -71,7 +77,7 @@ DisconnectRegistry(
  */
 NTSTATUS
 VerifyRegistryHive(
-    // IN HKEY RootKey OPTIONAL,
+    // IN HANDLE RootKey OPTIONAL,
     // // IN HANDLE RootDirectory OPTIONAL,
     IN PUNICODE_STRING NtSystemRoot,
     IN PCWSTR RegistryKey /* ,