[ros-diffs] [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=…
==============================================================================
--- 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) ||