[ros-diffs] [arty] 22684: Fixed reference counting in CmiConnectHive and CmiDisconnectHive. No longer need hacks to check reference counts. Deleted a ton of wierd code. Fixed bug where we allocated uninitialized memory for child nodes we never populated. Now reference counting mirrors pointers exactly: - Hold one reference for the parent key pointer - Hold one reference for the list entry in the connected hive list
arty at svn.reactos.org
arty at svn.reactos.org
Thu Jun 29 05:48:44 CEST 2006
Author: arty
Date: Thu Jun 29 07:48:43 2006
New Revision: 22684
URL: http://svn.reactos.org/svn/reactos?rev=22684&view=rev
Log:
Fixed reference counting in CmiConnectHive and CmiDisconnectHive.
No longer need hacks to check reference counts.
Deleted a ton of wierd code.
Fixed bug where we allocated uninitialized memory for child nodes we never
populated.
Now reference counting mirrors pointers exactly:
- Hold one reference for the parent key pointer
- Hold one reference for the list entry in the connected hive list
Modified:
trunk/reactos/ntoskrnl/cm/cm.h
trunk/reactos/ntoskrnl/cm/import.c
trunk/reactos/ntoskrnl/cm/registry.c
trunk/reactos/ntoskrnl/cm/regobj.c
Modified: trunk/reactos/ntoskrnl/cm/cm.h
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/cm.h?rev=22684&r1=22683&r2=22684&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/cm/cm.h (original)
+++ trunk/reactos/ntoskrnl/cm/cm.h Thu Jun 29 07:48:43 2006
@@ -358,6 +358,9 @@
/* Time stamp for the last access by the parse routine */
ULONG TimeStamp;
+
+ /* List entry for connected hives */
+ LIST_ENTRY HiveList;
} KEY_OBJECT, *PKEY_OBJECT;
/* Bits 31-22 (top 10 bits) of the cell index is the directory index */
Modified: trunk/reactos/ntoskrnl/cm/import.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/import.c?rev=22684&r1=22683&r2=22684&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/cm/import.c (original)
+++ trunk/reactos/ntoskrnl/cm/import.c Thu Jun 29 07:48:43 2006
@@ -142,6 +142,8 @@
/* Acquire hive list lock exclusively */
KeEnterCriticalRegion();
ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
+
+ DPRINT1("Adding new hive\n");
/* Add the new hive to the hive list */
InsertTailList(&CmiHiveListHead, &Hive->HiveList);
@@ -195,7 +197,6 @@
if (!NT_SUCCESS(Status))
{
DPRINT1 ("CmiConnectHive(%wZ) failed (Status %lx)\n", &KeyName, Status);
-// CmiRemoveRegistryHive(RegistryHive);
return FALSE;
}
Modified: trunk/reactos/ntoskrnl/cm/registry.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/registry.c?rev=22684&r1=22683&r2=22684&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/cm/registry.c (original)
+++ trunk/reactos/ntoskrnl/cm/registry.c Thu Jun 29 07:48:43 2006
@@ -36,6 +36,7 @@
KTIMER CmiWorkerTimer;
LIST_ENTRY CmiKeyObjectListHead;
+LIST_ENTRY CmiConnectedHiveList;
ULONG CmiTimer = 0;
volatile BOOLEAN CmiHiveSyncEnabled = FALSE;
@@ -385,6 +386,7 @@
/* Initialize the key object list */
InitializeListHead(&CmiKeyObjectListHead);
+ InitializeListHead(&CmiConnectedHiveList);
/* Initialize the worker timer */
KeInitializeTimerEx(&CmiWorkerTimer, SynchronizationTimer);
@@ -694,92 +696,94 @@
return Status;
}
-
NTSTATUS
CmiConnectHive(IN POBJECT_ATTRIBUTES KeyObjectAttributes,
IN PREGISTRY_HIVE RegistryHive)
{
- UNICODE_STRING RemainingPath;
- PKEY_OBJECT ParentKey;
- PKEY_OBJECT NewKey;
- NTSTATUS Status;
- PWSTR SubName;
- UNICODE_STRING ObjectName;
- OBJECT_CREATE_INFORMATION ObjectCreateInfo;
-
- DPRINT("CmiConnectHive(%p, %p) called.\n",
- KeyObjectAttributes, RegistryHive);
-
- /* Capture all the info */
- DPRINT("Capturing Create Info\n");
- Status = ObpCaptureObjectAttributes(KeyObjectAttributes,
- KernelMode,
- FALSE,
- &ObjectCreateInfo,
- &ObjectName);
- if (!NT_SUCCESS(Status))
- {
+ UNICODE_STRING RemainingPath;
+ PKEY_OBJECT ParentKey;
+ PKEY_OBJECT NewKey;
+ NTSTATUS Status;
+ PWSTR SubName;
+ UNICODE_STRING ObjectName;
+ OBJECT_CREATE_INFORMATION ObjectCreateInfo;
+
+ DPRINT("CmiConnectHive(%p, %p) called.\n",
+ KeyObjectAttributes, RegistryHive);
+
+ /* Capture all the info */
+ DPRINT("Capturing Create Info\n");
+ Status = ObpCaptureObjectAttributes(KeyObjectAttributes,
+ KernelMode,
+ FALSE,
+ &ObjectCreateInfo,
+ &ObjectName);
+
+ if (!NT_SUCCESS(Status))
+ {
DPRINT("ObpCaptureObjectAttributes() failed (Status %lx)\n", Status);
return Status;
- }
-
- Status = CmFindObject(&ObjectCreateInfo,
- &ObjectName,
- (PVOID*)&ParentKey,
- &RemainingPath,
- CmiKeyType,
- NULL,
- NULL);
- ObpReleaseCapturedAttributes(&ObjectCreateInfo);
- if (ObjectName.Buffer) ObpReleaseCapturedName(&ObjectName);
- if (!NT_SUCCESS(Status))
- {
- return Status;
- }
-
- DPRINT ("RemainingPath %wZ\n", &RemainingPath);
-
- if ((RemainingPath.Buffer == NULL) || (RemainingPath.Buffer[0] == 0))
- {
- ObDereferenceObject (ParentKey);
- RtlFreeUnicodeString(&RemainingPath);
- return STATUS_OBJECT_NAME_COLLISION;
- }
-
- /* Ignore leading backslash */
- SubName = RemainingPath.Buffer;
- if (*SubName == L'\\')
- SubName++;
-
- /* If RemainingPath contains \ we must return error
- because CmiConnectHive() can not create trees */
- if (wcschr (SubName, L'\\') != NULL)
- {
- ObDereferenceObject (ParentKey);
- RtlFreeUnicodeString(&RemainingPath);
- return STATUS_OBJECT_NAME_NOT_FOUND;
- }
-
- DPRINT("RemainingPath %wZ ParentKey %p\n",
- &RemainingPath, ParentKey);
-
- Status = ObCreateObject(KernelMode,
+ }
+
+ Status = CmFindObject(&ObjectCreateInfo,
+ &ObjectName,
+ (PVOID*)&ParentKey,
+ &RemainingPath,
CmiKeyType,
NULL,
- KernelMode,
- NULL,
- sizeof(KEY_OBJECT),
- 0,
- 0,
- (PVOID*)&NewKey);
-
- if (!NT_SUCCESS(Status))
- {
- DPRINT1 ("ObCreateObject() failed (Status %lx)\n", Status);
- ObDereferenceObject (ParentKey);
- RtlFreeUnicodeString(&RemainingPath);
- return Status;
- }
+ NULL);
+ /* Yields a new reference */
+ ObpReleaseCapturedAttributes(&ObjectCreateInfo);
+
+ if (ObjectName.Buffer) ObpReleaseCapturedName(&ObjectName);
+ if (!NT_SUCCESS(Status))
+ {
+ return Status;
+ }
+
+ DPRINT ("RemainingPath %wZ\n", &RemainingPath);
+
+ if ((RemainingPath.Buffer == NULL) || (RemainingPath.Buffer[0] == 0))
+ {
+ ObDereferenceObject (ParentKey);
+ RtlFreeUnicodeString(&RemainingPath);
+ return STATUS_OBJECT_NAME_COLLISION;
+ }
+
+ /* Ignore leading backslash */
+ SubName = RemainingPath.Buffer;
+ if (*SubName == L'\\')
+ SubName++;
+
+ /* If RemainingPath contains \ we must return error
+ because CmiConnectHive() can not create trees */
+ if (wcschr (SubName, L'\\') != NULL)
+ {
+ ObDereferenceObject (ParentKey);
+ RtlFreeUnicodeString(&RemainingPath);
+ return STATUS_OBJECT_NAME_NOT_FOUND;
+ }
+
+ DPRINT("RemainingPath %wZ ParentKey %p\n",
+ &RemainingPath, ParentKey);
+
+ Status = ObCreateObject(KernelMode,
+ CmiKeyType,
+ NULL,
+ KernelMode,
+ NULL,
+ sizeof(KEY_OBJECT),
+ 0,
+ 0,
+ (PVOID*)&NewKey);
+
+ if (!NT_SUCCESS(Status))
+ {
+ DPRINT1 ("ObCreateObject() failed (Status %lx)\n", Status);
+ ObDereferenceObject (ParentKey);
+ RtlFreeUnicodeString(&RemainingPath);
+ return Status;
+ }
DPRINT("Inserting Key into Object Tree\n");
Status = ObInsertObject((PVOID)NewKey,
NULL,
@@ -787,56 +791,42 @@
0,
NULL,
NULL);
-DPRINT("Status %x\n", Status);
- NewKey->RegistryHive = RegistryHive;
- NewKey->KeyCellOffset = RegistryHive->HiveHeader->RootKeyOffset;
- NewKey->KeyCell = CmiGetCell (RegistryHive, NewKey->KeyCellOffset, NULL);
- NewKey->Flags = 0;
- NewKey->NumberOfSubKeys = 0;
- InsertTailList(&CmiKeyObjectListHead, &NewKey->ListEntry);
- if (NewKey->KeyCell->NumberOfSubKeys != 0)
- {
- NewKey->SubKeys = ExAllocatePool(NonPagedPool,
- NewKey->KeyCell->NumberOfSubKeys * sizeof(ULONG));
- if (NewKey->SubKeys == NULL)
- {
- DPRINT("ExAllocatePool() failed\n");
- ObDereferenceObject (NewKey);
- ObDereferenceObject (ParentKey);
- RtlFreeUnicodeString(&RemainingPath);
- return STATUS_INSUFFICIENT_RESOURCES;
- }
- }
- else
- {
- NewKey->SubKeys = NULL;
- }
-
- DPRINT ("SubName %S\n", SubName);
-
- Status = RtlpCreateUnicodeString(&NewKey->Name,
- SubName, NonPagedPool);
- RtlFreeUnicodeString(&RemainingPath);
- if (!NT_SUCCESS(Status))
- {
- DPRINT1("RtlpCreateUnicodeString() failed (Status %lx)\n", Status);
- if (NewKey->SubKeys != NULL)
- {
- ExFreePool (NewKey->SubKeys);
- }
- ObDereferenceObject (NewKey);
- ObDereferenceObject (ParentKey);
- return STATUS_INSUFFICIENT_RESOURCES;
- }
-
- CmiAddKeyToList (ParentKey, NewKey);
- ObDereferenceObject (ParentKey);
-
- VERIFY_KEY_OBJECT(NewKey);
-
- /* Note: Do not dereference NewKey here! */
-
- return STATUS_SUCCESS;
+ DPRINT("Status %x\n", Status);
+ NewKey->RegistryHive = RegistryHive;
+ NewKey->KeyCellOffset = RegistryHive->HiveHeader->RootKeyOffset;
+ NewKey->KeyCell = CmiGetCell (RegistryHive, NewKey->KeyCellOffset, NULL);
+ NewKey->Flags = 0;
+ NewKey->NumberOfSubKeys = 0;
+ NewKey->SubKeys = NULL;
+ InsertTailList(&CmiKeyObjectListHead, &NewKey->ListEntry);
+ InsertTailList(&CmiConnectedHiveList, &NewKey->HiveList);
+
+ DPRINT ("SubName %S\n", SubName);
+
+ Status = RtlpCreateUnicodeString(&NewKey->Name,
+ SubName, NonPagedPool);
+ RtlFreeUnicodeString(&RemainingPath);
+ if (!NT_SUCCESS(Status))
+ {
+ DPRINT1("RtlpCreateUnicodeString() failed (Status %lx)\n", Status);
+ if (NewKey->SubKeys != NULL)
+ {
+ ExFreePool (NewKey->SubKeys);
+ }
+ ObDereferenceObject (NewKey);
+ ObDereferenceObject (ParentKey);
+ return STATUS_INSUFFICIENT_RESOURCES;
+ }
+
+ CmiAddKeyToList (ParentKey, NewKey);
+
+ VERIFY_KEY_OBJECT(NewKey);
+
+ /* We're holding a pointer to the parent key .. We must keep it
+ * referenced */
+ /* Note: Do not dereference NewKey here! */
+
+ return STATUS_SUCCESS;
}
@@ -845,9 +835,8 @@
OUT PREGISTRY_HIVE *RegistryHive)
{
PKEY_OBJECT KeyObject;
- PREGISTRY_HIVE Hive;
HANDLE KeyHandle;
- NTSTATUS Status;
+ NTSTATUS Status = STATUS_OBJECT_NAME_NOT_FOUND;
PLIST_ENTRY CurrentEntry;
PKEY_OBJECT CurrentKey;
@@ -875,68 +864,47 @@
(PVOID*)&KeyObject,
NULL);
ZwClose (KeyHandle);
+
if (!NT_SUCCESS(Status))
{
DPRINT1 ("ObReferenceObjectByName() failed (Status %lx)\n", Status);
return Status;
}
DPRINT ("KeyObject %p Hive %p\n", KeyObject, KeyObject->RegistryHive);
-
- if (!(KeyObject->KeyCell->Flags & REG_KEY_ROOT_CELL))
- {
- DPRINT1 ("Key is not the Hive-Root-Key\n");
- ObDereferenceObject (KeyObject);
- return STATUS_INVALID_PARAMETER;
- }
/* Acquire registry lock exclusively */
KeEnterCriticalRegion();
ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
- CurrentEntry = CmiKeyObjectListHead.Flink;
- while (CurrentEntry != &CmiKeyObjectListHead)
- {
- CurrentKey = CONTAINING_RECORD(CurrentEntry, KEY_OBJECT, ListEntry);
- if (1 == ObGetObjectPointerCount(CurrentKey) &&
- !(CurrentKey->Flags & KO_MARKED_FOR_DELETE))
- {
- ObDereferenceObject(CurrentKey);
- CurrentEntry = CmiKeyObjectListHead.Flink;
- }
- else
- {
- CurrentEntry = CurrentEntry->Flink;
- }
+ /* Find out if we represent a connected hive. */
+ for( CurrentEntry = CmiConnectedHiveList.Flink;
+ CurrentEntry != &CmiConnectedHiveList;
+ CurrentEntry = CurrentEntry->Flink ) {
+ CurrentKey = CONTAINING_RECORD(CurrentEntry, KEY_OBJECT, HiveList);
+ if( CurrentKey == KeyObject ) {
+ /* Remove the connected hive from the connected hive list */
+ RemoveEntryList(CurrentEntry);
+ /* found ourselves in the connected hive list */
+ *RegistryHive = KeyObject->RegistryHive;
+ Status = STATUS_SUCCESS;
+
+ /* Release references captured in CmiConnectHive */
+ ObDereferenceObject (KeyObject->ParentKey);
+ ObDereferenceObject (KeyObject);
+ break;
+ }
}
-
- if (ObGetObjectHandleCount (KeyObject) != 0 ||
- ObGetObjectPointerCount (KeyObject) != 2)
- {
- DPRINT1 ("Hive is still in use (hc %d, rc %d)\n", ObGetObjectHandleCount (KeyObject), ObGetObjectPointerCount (KeyObject));
- ObDereferenceObject (KeyObject);
-
- /* Release registry lock */
- ExReleaseResourceLite (&CmiRegistryLock);
- KeLeaveCriticalRegion();
-
- return STATUS_UNSUCCESSFUL;
- }
-
- Hive = KeyObject->RegistryHive;
-
- /* Dereference KeyObject twice to delete it */
- ObDereferenceObject (KeyObject);
- ObDereferenceObject (KeyObject);
-
- *RegistryHive = Hive;
-
+
/* Release registry lock */
ExReleaseResourceLite (&CmiRegistryLock);
KeLeaveCriticalRegion();
+ /* Release reference above */
+ ObDereferenceObject (KeyObject);
+
DPRINT ("CmiDisconnectHive() done\n");
- return STATUS_SUCCESS;
+ return Status;
}
Modified: trunk/reactos/ntoskrnl/cm/regobj.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/cm/regobj.c?rev=22684&r1=22683&r2=22684&view=diff
==============================================================================
--- trunk/reactos/ntoskrnl/cm/regobj.c (original)
+++ trunk/reactos/ntoskrnl/cm/regobj.c Thu Jun 29 07:48:43 2006
@@ -462,7 +462,7 @@
ExReleaseResourceLite(&CmiRegistryLock);
KeLeaveCriticalRegion();
- DPRINT("CmiObjectParse: %s\n", FoundObject->Name);
+ //DPRINT("CmiObjectParse: %s\n", FoundObject->Name);
*Path = EndPtr;
@@ -492,9 +492,9 @@
KeEnterCriticalRegion();
ExAcquireResourceExclusiveLite(&CmiRegistryLock, TRUE);
- //if (!NT_SUCCESS(CmiRemoveKeyFromList(KeyObject)))
- {
- // DPRINT1("Key not found in parent list ???\n");
+ if (!NT_SUCCESS(CmiRemoveKeyFromList(KeyObject)))
+ {
+ DPRINT1("Key not found in parent list ???\n");
}
RemoveEntryList(&KeyObject->ListEntry);
@@ -526,7 +526,7 @@
if (KeyObject->NumberOfSubKeys)
{
- //KEBUGCHECK(REGISTRY_ERROR);
+ KEBUGCHECK(REGISTRY_ERROR);
}
if (KeyObject->SizeOfSubKeys)
More information about the Ros-diffs
mailing list