[ros-diffs] [fireball] 38272: - Fix registry lock leakages in CmpDoOpen in failure cases. - Add support for a special case of opening existing (cached) KCBs by CmpDoOpen. - Specify a flag for that case in one specific case inside CmpParseKey, which allows us to get rid of the "\\REGISTRY" hack inside the code, fixes the 'Kcb->RefCount == 1' assertion failure, and certainly fixes registry-related regression tests. There might be some side effects in theory, but since this hack only applied to the root key, t

Author: fireball
Date: Mon Dec 22 12:47:38 2008
New Revision: 38272

URL: http://svn.reactos.org/svn/reactos?rev=38272&view=rev
Log:
- Fix registry lock leakages in CmpDoOpen in failure cases.
- Add support for a special case of opening existing (cached) KCBs by CmpDoOpen.
- Specify a flag for that case in one specific case inside CmpParseKey, which allows us to
get rid of the "\\REGISTRY" hack inside the code, fixes the
'Kcb->RefCount == 1' assertion failure, and certainly fixes registry-related
regression tests. There might be some side effects in theory, but since this hack only
applied to the root key, they shouldn't happen. Thanks to Stefan for helping in
debugging and a testcase.

Modified:
    trunk/reactos/ntoskrnl/config/cmparse.c

Modified: trunk/reactos/ntoskrnl/config/cmparse.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/config/cmparse.c?…
==============================================================================
--- trunk/reactos/ntoskrnl/config/cmparse.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/config/cmparse.c [iso-8859-1] Mon Dec 22 12:47:38 2008
@@ -599,37 +599,73 @@
     /* If we have a KCB, make sure it's locked */
     //ASSERT(CmpIsKcbLockedExclusive(*CachedKcb));
 
-    /* Check if this is a symlink */
-    if ((Node->Flags & KEY_SYM_LINK) && !(Attributes & OBJ_OPENLINK))
-    {
-        /* Create the KCB for the symlink */
+    /* Check if caller doesn't want to create a KCB */
+    if (ControlFlags & CMP_OPEN_KCB_NO_CREATE)
+    {
+        /* Check if this is a symlink */
+        if ((Node->Flags & KEY_SYM_LINK) && !(Attributes &
OBJ_OPENLINK))
+        {
+            /* This case for a cached KCB is not implemented yet */
+            ASSERT(FALSE);
+        }
+
+        /* The caller wants to open a cached KCB */
+        if (!CmpReferenceKeyControlBlock(*CachedKcb))
+        {
+            /* Release the registry lock */
+            CmpUnlockRegistry();
+
+            /* Return failure code */
+            return STATUS_INSUFFICIENT_RESOURCES;
+        }
+
+        /* Our kcb is that one */
+        Kcb = *CachedKcb;
+    }
+    else
+    {
+        /* Check if this is a symlink */
+        if ((Node->Flags & KEY_SYM_LINK) && !(Attributes &
OBJ_OPENLINK))
+        {
+            /* Create the KCB for the symlink */
+            Kcb = CmpCreateKeyControlBlock(Hive,
+                                           Cell,
+                                           Node,
+                                           *CachedKcb,
+                                           0,
+                                           KeyName);
+            if (!Kcb)
+            {
+                /* Release registry lock and return failure */
+                CmpUnlockRegistry();
+                return STATUS_INSUFFICIENT_RESOURCES;
+            }
+
+            /* Make sure it's also locked, and set the pointer */
+            //ASSERT(CmpIsKcbLockedExclusive(Kcb));
+            *CachedKcb = Kcb;
+
+            /* Release the registry lock */
+            CmpUnlockRegistry();
+
+            /* Return reparse required */
+            return STATUS_REPARSE;
+        }
+
+        /* Create the KCB. FIXME: Use lock flag */
         Kcb = CmpCreateKeyControlBlock(Hive,
                                        Cell,
                                        Node,
                                        *CachedKcb,
                                        0,
                                        KeyName);
-        if (!Kcb) return STATUS_INSUFFICIENT_RESOURCES;
-
-        /* Make sure it's also locked, and set the pointer */
-        //ASSERT(CmpIsKcbLockedExclusive(Kcb));
-        *CachedKcb = Kcb;
-
-        /* Release the registry lock */
-        CmpUnlockRegistry();
-
-        /* Return reparse required */
-        return STATUS_REPARSE;
-    }
-
-    /* Create the KCB. FIXME: Use lock flag */
-    Kcb = CmpCreateKeyControlBlock(Hive,
-                                   Cell,
-                                   Node,
-                                   *CachedKcb,
-                                   0,
-                                   KeyName);
-    if (!Kcb) return STATUS_INSUFFICIENT_RESOURCES;
+        if (!Kcb)
+        {
+            /* Release registry lock and return failure */
+            CmpUnlockRegistry();
+            return STATUS_INSUFFICIENT_RESOURCES;
+        }
+    }
 
     /* Make sure it's also locked, and set the pointer */
     //ASSERT(CmpIsKcbLockedExclusive(Kcb));
@@ -1308,10 +1344,7 @@
                                   &CellToRelease);
                 if (!Node) ASSERT(FALSE);
             }
-            
-            /* FIXME: This hack seems required? */
-            RtlInitUnicodeString(&NextName, L"\\REGISTRY");
-            
+
             /* Do the open */
             Status = CmpDoOpen(Hive,
                                Cell,
@@ -1320,7 +1353,7 @@
                                AccessMode,
                                Attributes,
                                ParseContext,
-                               0,
+                               CMP_OPEN_KCB_NO_CREATE /* | CMP_CREATE_KCB_KCB_LOCKED */,
                                &Kcb,
                                &NextName,
                                Object);