[sginsberg] 35633: - Remove LIST_FOR_EACH and LIST_FOR_EACH_SAFE from the kernel - Fix a bug in KeStartProfile (a missing negation) which caused us to either free buffer when we shouldn't, or leak it
Author: sginsberg Date: Mon Aug 25 13:21:19 2008 New Revision: 35633
URL: http://svn.reactos.org/svn/reactos?rev=35633&view=rev Log: - Remove LIST_FOR_EACH and LIST_FOR_EACH_SAFE from the kernel - Fix a bug in KeStartProfile (a missing negation) which caused us to either free buffer when we shouldn't, or leak it
Modified: trunk/reactos/ntoskrnl/io/iomgr/drvrlist.c trunk/reactos/ntoskrnl/io/pnpmgr/pnproot.c trunk/reactos/ntoskrnl/ke/profobj.c
Modified: trunk/reactos/ntoskrnl/io/iomgr/drvrlist.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/iomgr/drvrlist.... ============================================================================== --- trunk/reactos/ntoskrnl/io/iomgr/drvrlist.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/io/iomgr/drvrlist.c [iso-8859-1] Mon Aug 25 13:21:19 2008 @@ -350,14 +350,22 @@ NTSTATUS INIT_FUNCTION IoDestroyDriverList(VOID) { - PSERVICE_GROUP CurrentGroup, tmp1; - PSERVICE CurrentService, tmp2; + PSERVICE_GROUP CurrentGroup; + PSERVICE CurrentService; + PLIST_ENTRY NextEntry, TempEntry;
DPRINT("IoDestroyDriverList() called\n");
/* Destroy the Group List */ - LIST_FOR_EACH_SAFE(CurrentGroup, tmp1, &GroupListHead, SERVICE_GROUP, GroupListEntry) - { + for (NextEntry = GroupListHead.Flink, TempEntry = NextEntry->Flink; + NextEntry != &GroupListHead; + NextEntry = TempEntry, TempEntry = NextEntry->Flink) + { + /* Get the entry */ + CurrentGroup = CONTAINING_RECORD(NextEntry, + SERVICE_GROUP, + GroupListEntry); + /* Remove it from the list */ RemoveEntryList(&CurrentGroup->GroupListEntry);
@@ -369,8 +377,15 @@ }
/* Destroy the Service List */ - LIST_FOR_EACH_SAFE(CurrentService, tmp2, &ServiceListHead, SERVICE, ServiceListEntry) - { + for (NextEntry = ServiceListHead.Flink, TempEntry = NextEntry->Flink; + NextEntry != &ServiceListHead; + NextEntry = TempEntry, TempEntry = NextEntry->Flink) + { + /* Get the entry */ + CurrentService = CONTAINING_RECORD(NextEntry, + SERVICE, + ServiceListEntry); + /* Remove it from the list */ RemoveEntryList(&CurrentService->ServiceListEntry);
@@ -448,21 +463,35 @@ PSERVICE CurrentService; NTSTATUS Status; ULONG i; + PLIST_ENTRY NextGroupEntry, NextServiceEntry;
DPRINT("IopInitializeSystemDrivers()\n");
- /* Loop the list */ - LIST_FOR_EACH(CurrentGroup, &GroupListHead, SERVICE_GROUP, GroupListEntry) - { + /* Start looping */ + for (NextGroupEntry = GroupListHead.Flink; + NextGroupEntry != &GroupListHead; + NextGroupEntry = NextGroupEntry->Flink) + { + /* Get the entry */ + CurrentGroup = CONTAINING_RECORD(NextGroupEntry, + SERVICE_GROUP, + GroupListEntry);
DPRINT("Group: %wZ\n", &CurrentGroup->GroupName);
/* Load all drivers with a valid tag */ for (i = 0; i < CurrentGroup->TagCount; i++) { - /* Loop the list */ - LIST_FOR_EACH(CurrentService, &ServiceListHead, SERVICE, ServiceListEntry) + /* Start looping */ + for (NextServiceEntry = ServiceListHead.Flink; + NextServiceEntry != &ServiceListHead; + NextServiceEntry = NextServiceEntry->Flink) { + /* Get the entry */ + CurrentService = CONTAINING_RECORD(NextServiceEntry, + SERVICE, + ServiceListEntry); + if ((!RtlCompareUnicodeString(&CurrentGroup->GroupName, &CurrentService->ServiceGroup, TRUE)) && @@ -477,8 +506,15 @@ }
/* Load all drivers without a tag or with an invalid tag */ - LIST_FOR_EACH(CurrentService, &ServiceListHead, SERVICE, ServiceListEntry) + for (NextServiceEntry = ServiceListHead.Flink; + NextServiceEntry != &ServiceListHead; + NextServiceEntry = NextServiceEntry->Flink) { + /* Get the entry */ + CurrentService = CONTAINING_RECORD(NextServiceEntry, + SERVICE, + ServiceListEntry); + if ((!RtlCompareUnicodeString(&CurrentGroup->GroupName, &CurrentService->ServiceGroup, TRUE)) &&
Modified: trunk/reactos/ntoskrnl/io/pnpmgr/pnproot.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/io/pnpmgr/pnproot.... ============================================================================== --- trunk/reactos/ntoskrnl/io/pnpmgr/pnproot.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/io/pnpmgr/pnproot.c [iso-8859-1] Mon Aug 25 13:21:19 2008 @@ -99,14 +99,20 @@ { PPNPROOT_DEVICE Device; UNICODE_STRING DeviceIdU, InstanceIdU; + PLIST_ENTRY NextEntry;
/* Initialize the strings to compare */ RtlInitUnicodeString(&DeviceIdU, DeviceId); RtlInitUnicodeString(&InstanceIdU, InstanceId);
/* Start looping */ - LIST_FOR_EACH(Device, &DeviceExtension->DeviceListHead, PNPROOT_DEVICE, ListEntry) - { + for (NextEntry = DeviceExtension->DeviceListHead.Flink; + NextEntry != &DeviceExtension->DeviceListHead; + NextEntry = NextEntry->Flink) + { + /* Get the entry */ + Device = CONTAINING_RECORD(NextEntry, PNPROOT_DEVICE, ListEntry); + /* See if the strings match */ if (RtlEqualUnicodeString(&DeviceIdU, &Device->DeviceID, TRUE) && RtlEqualUnicodeString(&InstanceIdU, &Device->InstanceID, TRUE)) @@ -521,6 +527,7 @@ PPNPROOT_DEVICE Device = NULL; ULONG Size; NTSTATUS Status; + PLIST_ENTRY NextEntry;
DPRINT("PnpRootQueryDeviceRelations(FDO %p, Irp %p)\n", DeviceObject, Irp);
@@ -556,8 +563,15 @@ }
KeAcquireGuardedMutex(&DeviceExtension->DeviceListLock); - LIST_FOR_EACH(Device, &DeviceExtension->DeviceListHead, PNPROOT_DEVICE, ListEntry) - { + + /* Start looping */ + for (NextEntry = DeviceExtension->DeviceListHead.Flink; + NextEntry != &DeviceExtension->DeviceListHead; + NextEntry = NextEntry->Flink) + { + /* Get the entry */ + Device = CONTAINING_RECORD(NextEntry, PNPROOT_DEVICE, ListEntry); + if (!Device->Pdo) { /* Create a physical device object for the
Modified: trunk/reactos/ntoskrnl/ke/profobj.c URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/profobj.c?rev=3... ============================================================================== --- trunk/reactos/ntoskrnl/ke/profobj.c [iso-8859-1] (original) +++ trunk/reactos/ntoskrnl/ke/profobj.c [iso-8859-1] Mon Aug 25 13:21:19 2008 @@ -54,8 +54,9 @@ KIRQL OldIrql; PKPROFILE_SOURCE_OBJECT SourceBuffer; PKPROFILE_SOURCE_OBJECT CurrentSource; - BOOLEAN FreeBuffer = TRUE; + BOOLEAN FreeBuffer = TRUE, SourceFound = FALSE;; PKPROCESS ProfileProcess; + PLIST_ENTRY NextEntry;
/* Allocate a buffer first, before we raise IRQL */ SourceBuffer = ExAllocatePoolWithTag(NonPagedPool, @@ -89,18 +90,27 @@ InsertTailList(&KiProfileListHead, &Profile->ProfileListEntry); }
- /* Check if this type of profile (source) is already running */ - LIST_FOR_EACH(CurrentSource, &KiProfileSourceListHead, KPROFILE_SOURCE_OBJECT, ListEntry) - { + /* Start looping */ + for (NextEntry = KiProfileSourceListHead.Flink; + NextEntry != &KiProfileSourceListHead; + NextEntry = NextEntry->Flink) + { + /* Get the entry */ + CurrentSource = CONTAINING_RECORD(NextEntry, + KPROFILE_SOURCE_OBJECT, + ListEntry); + /* Check if it's the same as the one being requested now */ if (CurrentSource->Source == Profile->Source) { + /* It is, break out */ + SourceFound = TRUE; break; } }
/* See if the loop found something */ - if (!CurrentSource) + if (!SourceFound) { /* Nothing found, use our allocated buffer */ CurrentSource = SourceBuffer; @@ -122,7 +132,7 @@ //HalStartProfileInterrupt(Profile->Source);
/* Free the pool */ - if (!FreeBuffer) ExFreePool(SourceBuffer); + if (FreeBuffer) ExFreePool(SourceBuffer); }
BOOLEAN @@ -131,6 +141,8 @@ { KIRQL OldIrql; PKPROFILE_SOURCE_OBJECT CurrentSource = NULL; + PLIST_ENTRY NextEntry; + BOOLEAN SourceFound = FALSE;
/* Raise to PROFILE_LEVEL and acquire spinlock */ KeRaiseIrql(PROFILE_LEVEL, &OldIrql); @@ -143,12 +155,23 @@ RemoveEntryList(&Profile->ProfileListEntry); Profile->Started = FALSE;
- /* Find the Source Object */ - LIST_FOR_EACH(CurrentSource, &KiProfileSourceListHead, KPROFILE_SOURCE_OBJECT, ListEntry) - { + /* Start looping */ + for (NextEntry = KiProfileSourceListHead.Flink; + NextEntry != &KiProfileSourceListHead; + NextEntry = NextEntry->Flink) + { + /* Get the entry */ + CurrentSource = CONTAINING_RECORD(NextEntry, + KPROFILE_SOURCE_OBJECT, + ListEntry); + + /* Check if this is the Source Object */ if (CurrentSource->Source == Profile->Source) { - /* Remove it */ + /* Remember we found one */ + SourceFound = TRUE; + + /* Remove it and break out */ RemoveEntryList(&CurrentSource->ListEntry); break; } @@ -164,7 +187,7 @@ //HalStopProfileInterrupt(Profile->Source);
/* Free the Source Object */ - if (CurrentSource) ExFreePool(CurrentSource); + if (SourceFound) ExFreePool(CurrentSource);
/* FIXME */ return FALSE; @@ -231,10 +254,16 @@ { PULONG BucketValue; PKPROFILE Profile; + PLIST_ENTRY NextEntry;
/* Loop the List */ - LIST_FOR_EACH(Profile, ListHead, KPROFILE, ProfileListEntry) - { + for (NextEntry = ListHead->Flink; + NextEntry != ListHead; + NextEntry = NextEntry->Flink) + { + /* Get the entry */ + Profile = CONTAINING_RECORD(NextEntry, KPROFILE, ProfileListEntry); + /* Check if the source is good, and if it's within the range */ #ifdef _M_IX86 if ((Profile->Source != Source) ||
-
sginsberg＠svn.reactos.org