[ros-diffs] [hbelusca] 57806: [CSRSRV] - Comment on the size of some members of the CSR_WAIT_BLOCK structure. - Initialize the WaitBlock member of CSR_THREAD to a valid value when creating a wait block, and NU...

Author: hbelusca
Date: Wed Dec  5 23:21:41 2012
New Revision: 57806

URL: http://svn.reactos.org/svn/reactos?rev=57806&view=rev
Log:
[CSRSRV]
- Comment on the size of some members of the CSR_WAIT_BLOCK structure.
- Initialize the WaitBlock member of CSR_THREAD to a valid value when creating a wait
block, and NULLify it when we release a wait block.
- ALWAYS USE offsets in CSR_CAPTURE_BUFFER structure, instead of real pointers !! It is
needed when their base address change (eg. during a CSR wait, their base address,
corresponding to the address of an CSR API message, change) (found when testing CSR waits
with the console).

Modified:
    branches/ros-csrss/include/reactos/subsys/csr/csrsrv.h
    branches/ros-csrss/subsystems/win32/csrsrv/api.c
    branches/ros-csrss/subsystems/win32/csrsrv/wait.c

Modified: branches/ros-csrss/include/reactos/subsys/csr/csrsrv.h
URL:
http://svn.reactos.org/svn/reactos/branches/ros-csrss/include/reactos/subsy…
==============================================================================
--- branches/ros-csrss/include/reactos/subsys/csr/csrsrv.h [iso-8859-1] (original)
+++ branches/ros-csrss/include/reactos/subsys/csr/csrsrv.h [iso-8859-1] Wed Dec  5
23:21:41 2012
@@ -156,13 +156,13 @@
 
 typedef struct _CSR_WAIT_BLOCK
 {
-    ULONG Size;
+    ULONG Size;                     // Size of the wait block (variable-sized)
     LIST_ENTRY WaitList;
     LIST_ENTRY UserWaitList;
     PVOID WaitContext;
     PCSR_THREAD WaitThread;
     CSR_WAIT_FUNCTION WaitFunction;
-    CSR_API_MESSAGE WaitApiMessage;
+    CSR_API_MESSAGE WaitApiMessage; // Variable-sized CSR API message
 } CSR_WAIT_BLOCK, *PCSR_WAIT_BLOCK;
 
 

Modified: branches/ros-csrss/subsystems/win32/csrsrv/api.c
URL:
http://svn.reactos.org/svn/reactos/branches/ros-csrss/subsystems/win32/csrs…
==============================================================================
--- branches/ros-csrss/subsystems/win32/csrsrv/api.c [iso-8859-1] (original)
+++ branches/ros-csrss/subsystems/win32/csrsrv/api.c [iso-8859-1] Wed Dec  5 23:21:41
2012
@@ -504,7 +504,7 @@
                         if ((ServerDll) && (ServerDll->HardErrorCallback))
                         {
                             /* Call it */
-                            ServerDll->HardErrorCallback(NULL /* CsrThread == NULL */,
HardErrorMsg);
+                            ServerDll->HardErrorCallback(NULL /* == CsrThread */,
HardErrorMsg);
 
                             /* If it's handled, get out of here */
                             if (HardErrorMsg->Response != ResponseNotHandled) break;
@@ -524,6 +524,7 @@
                 else
                 {
                     ReplyMsg = &ReceiveMsg;
+                    ReplyPort = CsrApiPort;
                 }
             }
             else if (MessageType == LPC_REQUEST)
@@ -1178,20 +1179,21 @@
     BufferDistance = (ULONG_PTR)RemoteCaptureBuffer - (ULONG_PTR)LocalCaptureBuffer;
 
     /*
-     * Convert all the pointer offsets into real pointers, and make
-     * them point to the remote data buffer instead of the local one.
+     * All the pointer offsets correspond to pointers which point
+     * to the remote data buffer instead of the local one.
      */
     for (i = 0 ; i < RemoteCaptureBuffer->PointerCount ; ++i)
     {
         if (RemoteCaptureBuffer->PointerOffsetsArray[i] != 0)
         {
+            /* Temporarily transform the offset into a pointer */
             RemoteCaptureBuffer->PointerOffsetsArray[i] += (ULONG_PTR)ApiMessage;
 
-            /* Validate the bounds of the current pointer */
+            /* Validate the bounds of the current pointed pointer */
             if ((*(PULONG_PTR)RemoteCaptureBuffer->PointerOffsetsArray[i] >=
CsrThread->Process->ClientViewBase) &&
                 (*(PULONG_PTR)RemoteCaptureBuffer->PointerOffsetsArray[i] <
CsrThread->Process->ClientViewBounds))
             {
-                /* Modify the pointer to take into account its new position */
+                /* Modify the pointed pointer to take into account its new position */
                 *(PULONG_PTR)RemoteCaptureBuffer->PointerOffsetsArray[i] +=
BufferDistance;
             }
             else
@@ -1201,19 +1203,22 @@
                 DbgBreakPoint();
                 ApiMessage->Status = STATUS_INVALID_PARAMETER;
             }
+
+            /* Transform back into an offset */
+            RemoteCaptureBuffer->PointerOffsetsArray[i] -= (ULONG_PTR)ApiMessage;
         }
     }
 
     /* Check if we got success */
     if (ApiMessage->Status != STATUS_SUCCESS)
     {
-        /* Failure. Free the buffer and return*/
+        /* Failure. Free the buffer and return */
         RtlFreeHeap(CsrHeap, 0, RemoteCaptureBuffer);
         return FALSE;
     }
     else
     {
-        /* Success, save the previous buffer */
+        /* Success, save the previous buffer and use the remote capture buffer */
         RemoteCaptureBuffer->PreviousCaptureBuffer = LocalCaptureBuffer;
         ApiMessage->CsrCaptureData = RemoteCaptureBuffer;
     }
@@ -1246,29 +1251,38 @@
     SIZE_T BufferDistance;
     ULONG i;
 
-    /* Get the capture buffers */
+    /* Get the remote capture buffer */
     RemoteCaptureBuffer = ApiMessage->CsrCaptureData;
-    LocalCaptureBuffer = RemoteCaptureBuffer->PreviousCaptureBuffer;
 
     /* Do not continue if there is no captured buffer */
     if (!RemoteCaptureBuffer) return;
 
-    /* Free the previous one */
+    /* If there is one, get the corresponding local capture buffer */
+    LocalCaptureBuffer = RemoteCaptureBuffer->PreviousCaptureBuffer;
+
+    /* Free the previous one and use again the local capture buffer */
     RemoteCaptureBuffer->PreviousCaptureBuffer = NULL;
+    ApiMessage->CsrCaptureData = LocalCaptureBuffer;
 
     /* Calculate the difference between our buffer and the client's */
     BufferDistance = (ULONG_PTR)RemoteCaptureBuffer - (ULONG_PTR)LocalCaptureBuffer;
 
     /*
-     * Convert back all the pointers into pointer offsets, and make them
-     * point to the local data buffer instead of the remote one (revert
+     * All the pointer offsets correspond to pointers which point
+     * to the local data buffer instead of the remote one (revert
      * the logic of CsrCaptureArguments).
      */
     for (i = 0 ; i < RemoteCaptureBuffer->PointerCount ; ++i)
     {
         if (RemoteCaptureBuffer->PointerOffsetsArray[i] != 0)
         {
+            /* Temporarily transform the offset into a pointer */
+            RemoteCaptureBuffer->PointerOffsetsArray[i] += (ULONG_PTR)ApiMessage;
+
+            /* Modify the pointed pointer to take into account its new position */
             *(PULONG_PTR)RemoteCaptureBuffer->PointerOffsetsArray[i] -=
BufferDistance;
+
+            /* Transform back into an offset */
             RemoteCaptureBuffer->PointerOffsetsArray[i] -= (ULONG_PTR)ApiMessage;
         }
     }
@@ -1313,7 +1327,7 @@
                          IN ULONG ElementSize)
 {
     PCSR_CAPTURE_BUFFER CaptureBuffer = ApiMessage->CsrCaptureData;
-    // SIZE_T BufferDistance = (ULONG_PTR)Buffer - (ULONG_PTR)ApiMessage;
+    SIZE_T BufferDistance = (ULONG_PTR)Buffer - (ULONG_PTR)ApiMessage;
     ULONG i;
 
     /*
@@ -1353,10 +1367,10 @@
             for (i = 0 ; i < CaptureBuffer->PointerCount ; ++i)
             {
                 /*
-                 * If the pointer offset is in fact equal to the
-                 * real address of the buffer then it's OK.
+                 * The pointer offset must be equal to the delta between
+                 * the addresses of the buffer and of the API message.
                  */
-                if (CaptureBuffer->PointerOffsetsArray[i] == (ULONG_PTR)Buffer /*
BufferDistance + (ULONG_PTR)ApiMessage */)
+                if (CaptureBuffer->PointerOffsetsArray[i] == BufferDistance)
                 {
                     return TRUE;
                 }

Modified: branches/ros-csrss/subsystems/win32/csrsrv/wait.c
URL:
http://svn.reactos.org/svn/reactos/branches/ros-csrss/subsystems/win32/csrs…
==============================================================================
--- branches/ros-csrss/subsystems/win32/csrsrv/wait.c [iso-8859-1] (original)
+++ branches/ros-csrss/subsystems/win32/csrsrv/wait.c [iso-8859-1] Wed Dec  5 23:21:41
2012
@@ -73,6 +73,7 @@
     /* Initialize it */
     WaitBlock->Size = Size;
     WaitBlock->WaitThread = CsrWaitThread;
+    CsrWaitThread->WaitBlock = WaitBlock;
     WaitBlock->WaitContext = WaitContext;
     WaitBlock->WaitFunction = WaitFunction;
     WaitBlock->UserWaitList.Flink = NULL;
@@ -242,6 +243,7 @@
     if (CsrWaitThread->Flags & CsrThreadTerminated)
     {
         /* Fail the wait */
+        CsrWaitThread->WaitBlock = NULL;
         RtlFreeHeap(CsrHeap, 0, WaitBlock);
         CsrReleaseWaitLock();
         return FALSE;