[jgardou] 57798: [NTOSKRNL/fsrtl] - Remove the right element from the list when uninitializing a file lock - Do not modify the content of the table range while looping on its elements - Fix rebuild...

Author: jgardou
Date: Tue Dec  4 12:09:59 2012
New Revision: 57798

URL: http://svn.reactos.org/svn/reactos?rev=57798&view=rev
Log:
[NTOSKRNL/fsrtl]
- Remove the right element from the list when uninitializing a file lock
- Do not modify the content of the table range while looping on its elements
- Fix rebuilding the table when unlocking a shared range
Thanks go to Pierre for the hint for where to look at
CORE-6615 #comment fixed by r57798 #resolve
CIRE-6535 #comment fixed by r55798 #resolve

Modified:
    trunk/reactos/ntoskrnl/fsrtl/filelock.c

Modified: trunk/reactos/ntoskrnl/fsrtl/filelock.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/fsrtl/filelock.c?…
==============================================================================
--- trunk/reactos/ntoskrnl/fsrtl/filelock.c [iso-8859-1] (original)
+++ trunk/reactos/ntoskrnl/fsrtl/filelock.c [iso-8859-1] Tue Dec  4 12:09:59 2012
@@ -514,33 +514,30 @@
                         }
                         return FALSE;
                     }
-                    else
-                    {
-                        DPRINT("Overlapping shared lock %wZ %08x%08x
%08x%08x\n",
-                               &FileObject->FileName,
-                               Conflict->Exclusive.FileLock.StartingByte.HighPart,
-                               Conflict->Exclusive.FileLock.StartingByte.LowPart,
-                               Conflict->Exclusive.FileLock.EndingByte.HighPart,
-                               Conflict->Exclusive.FileLock.EndingByte.LowPart);
-                        Conflict = FsRtlpRebuildSharedLockRange(FileLock,
-                                                                LockInfo,
-                                                                &ToInsert);
-                        if (!Conflict)
-                        {
-                            IoStatus->Status = STATUS_NO_MEMORY;
-                            if (Irp)
-                            {
-                                FsRtlCompleteLockIrpReal
-                                    (FileLock->CompleteLockIrpRoutine,
-                                     Context,
-                                     Irp,
-                                     IoStatus->Status,
-                                     &Status,
-                                     FileObject);
-                            }
-                            break;
-                        }
-                    }
+                }
+            }
+            
+            DPRINT("Overlapping shared lock %wZ %08x%08x %08x%08x\n",
+                   &FileObject->FileName,
+                   Conflict->Exclusive.FileLock.StartingByte.HighPart,
+                   Conflict->Exclusive.FileLock.StartingByte.LowPart,
+                   Conflict->Exclusive.FileLock.EndingByte.HighPart,
+                   Conflict->Exclusive.FileLock.EndingByte.LowPart);
+            Conflict = FsRtlpRebuildSharedLockRange(FileLock,
+                                                    LockInfo,
+                                                    &ToInsert);
+            if (!Conflict)
+            {
+                IoStatus->Status = STATUS_NO_MEMORY;
+                if (Irp)
+                {
+                    FsRtlCompleteLockIrpReal
+                        (FileLock->CompleteLockIrpRoutine,
+                         Context,
+                         Irp,
+                         IoStatus->Status,
+                         &Status,
+                         FileObject);
                 }
             }
 
@@ -567,8 +564,8 @@
                 return FALSE;
             }
             DPRINT("Adding shared lock %wZ\n", &FileObject->FileName);
-            NewSharedRange->Start = ToInsert.Exclusive.FileLock.StartingByte;
-            NewSharedRange->End = ToInsert.Exclusive.FileLock.EndingByte;
+            NewSharedRange->Start = *FileOffset;
+            NewSharedRange->End.QuadPart = FileOffset->QuadPart +
Length->QuadPart;
             NewSharedRange->Key = Key;
             NewSharedRange->ProcessId = ToInsert.Exclusive.FileLock.ProcessId;
             InsertTailList(&LockInfo->SharedLocks,
&NewSharedRange->Entry);
@@ -639,10 +636,10 @@
                 return FALSE;
             }
             DPRINT("Adding shared lock %wZ\n", &FileObject->FileName);
-            NewSharedRange->Start = ToInsert.Exclusive.FileLock.StartingByte;
-            NewSharedRange->End = ToInsert.Exclusive.FileLock.EndingByte;
+            NewSharedRange->Start = *FileOffset;
+            NewSharedRange->End.QuadPart = FileOffset->QuadPart +
Length->QuadPart;
             NewSharedRange->Key = Key;
-            NewSharedRange->ProcessId = ToInsert.Exclusive.FileLock.ProcessId;
+            NewSharedRange->ProcessId = Process->UniqueProcessId;
             InsertTailList(&LockInfo->SharedLocks,
&NewSharedRange->Entry);
         }
         
@@ -918,12 +915,7 @@
         }
         if (FoundShared)
         {
-            PLIST_ENTRY SharedRangeEntry;
-            PLOCK_SHARED_RANGE WatchSharedRange;
-            COMBINED_LOCK_ELEMENT RemadeElement;
-            Find.Exclusive.FileLock.StartingByte = SharedRange->Start;
-            Find.Exclusive.FileLock.EndingByte = SharedRange->End;
-            SharedEntry = SharedRange->Entry.Flink;
+            /* Remove the found range from the shared range lists */
             RemoveEntryList(&SharedRange->Entry);
             ExFreePoolWithTag(SharedRange, 'FSRA');
             /* We need to rebuild the list of shared ranges. */
@@ -933,19 +925,25 @@
                    Entry->Exclusive.FileLock.StartingByte.LowPart,
                    Entry->Exclusive.FileLock.EndingByte.HighPart, 
                    Entry->Exclusive.FileLock.EndingByte.LowPart);
-            /* Copy */
-            RemadeElement = *Entry;
-            RtlDeleteElementGenericTable(&InternalInfo->RangeTable, Entry);
+               
+            /* Remember what was in there and remove it from the table */
+            Find = *Entry;
+            RtlDeleteElementGenericTable(&InternalInfo->RangeTable, &Find);
             /* Put shared locks back in place */
-            for (SharedRangeEntry = InternalInfo->SharedLocks.Flink;
-                 SharedRangeEntry != &InternalInfo->SharedLocks;
-                 SharedRangeEntry = SharedRangeEntry->Flink)
+            for (SharedEntry = InternalInfo->SharedLocks.Flink;
+                 SharedEntry != &InternalInfo->SharedLocks;
+                 SharedEntry = SharedEntry->Flink)
             {
                 COMBINED_LOCK_ELEMENT LockElement;
-                WatchSharedRange = CONTAINING_RECORD(SharedRangeEntry, LOCK_SHARED_RANGE,
Entry);
-                LockElement.Exclusive.FileLock.StartingByte =
WatchSharedRange->Start;
-                LockElement.Exclusive.FileLock.EndingByte = WatchSharedRange->End;
-                if (LockCompare(&InternalInfo->RangeTable, &RemadeElement,
&LockElement) != GenericEqual)
+                SharedRange = CONTAINING_RECORD(SharedEntry, LOCK_SHARED_RANGE, Entry);
+                LockElement.Exclusive.FileLock.FileObject = FileObject;
+                LockElement.Exclusive.FileLock.StartingByte = SharedRange->Start;
+                LockElement.Exclusive.FileLock.EndingByte = SharedRange->End;
+                LockElement.Exclusive.FileLock.ProcessId = SharedRange->ProcessId;
+                LockElement.Exclusive.FileLock.Key = SharedRange->Key;
+                LockElement.Exclusive.FileLock.ExclusiveLock = FALSE;
+                
+                if (LockCompare(&InternalInfo->RangeTable, &Find,
&LockElement) != GenericEqual)
                 {
                     DPRINT("Skipping range %08x%08x:%08x%08x\n",
                            LockElement.Exclusive.FileLock.StartingByte.HighPart,
@@ -959,10 +957,7 @@
                        LockElement.Exclusive.FileLock.StartingByte.LowPart,
                        LockElement.Exclusive.FileLock.EndingByte.HighPart,
                        LockElement.Exclusive.FileLock.EndingByte.LowPart);
-                RtlZeroMemory(&RemadeElement, sizeof(RemadeElement));
-                RemadeElement.Exclusive.FileLock.StartingByte =
WatchSharedRange->Start;
-                RemadeElement.Exclusive.FileLock.EndingByte = WatchSharedRange->End;
-                FsRtlpRebuildSharedLockRange(FileLock, InternalInfo,
&RemadeElement);
+                FsRtlpRebuildSharedLockRange(FileLock, InternalInfo, &LockElement);
             }
         }
         else
@@ -970,7 +965,8 @@
             return STATUS_RANGE_NOT_LOCKED;
         }
     }
-    
+
+#ifndef NDEBUG    
     DPRINT("Lock still has:\n");
     for (SharedEntry = InternalInfo->SharedLocks.Flink;
          SharedEntry != &InternalInfo->SharedLocks;
@@ -985,6 +981,7 @@
                SharedRange->End.LowPart,
                SharedRange->Key);
     }
+#endif
     
     // this is definitely the thing we want
     InternalInfo->Generation++;
@@ -1286,7 +1283,7 @@
         {
             SharedRange = CONTAINING_RECORD(SharedEntry, LOCK_SHARED_RANGE, Entry);
             SharedEntry = SharedEntry->Flink;
-            RemoveEntryList(SharedEntry);
+            RemoveEntryList(&SharedRange->Entry);
             ExFreePoolWithTag(SharedRange, 'FSRA');
         }
         while ((Entry = RtlGetElementGenericTable(&InternalInfo->RangeTable, 0))
!= NULL)