[ros-diffs] [cfinck] 68364: [LOCALMON] - Bugfix: LocalmonClosePort must only free the memory for virtual file ports. Other port entries are reused. - Bugfix: Set the port type to PortType_OtherLPT in all condi...

Author: cfinck
Date: Tue Jul  7 10:30:30 2015
New Revision: 68364

URL: http://svn.reactos.org/svn/reactos?rev=68364&view=rev
Log:
[LOCALMON]
- Bugfix: LocalmonClosePort must only free the memory for virtual file ports. Other port
entries are reused.
- Bugfix: Set the port type to PortType_OtherLPT in all conditions that ít's not a
physical LPT port.
- Synchronize access to the port lists.
- Keep a list of open Xcv handles as well to properly close them on shutdown.
- Add traces to all monitor functions for easier debugging. (requires
DEBUGCHANNEL=+localmon environment variable)

Modified:
   
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/main.c
   
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/ports.c
   
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/precomp.h
    branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/xcv.c

Modified:
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/main.c
URL:
http://svn.reactos.org/svn/reactos/branches/colins-printing-for-freedom/rea…
==============================================================================
---
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/main.c	[iso-8859-1]
(original)
+++
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/main.c	[iso-8859-1]
Tue Jul  7 10:30:30 2015
@@ -67,6 +67,9 @@
 {
     PLOCALMON_HANDLE pLocalmon;
     PLOCALMON_PORT pPort;
+    PLOCALMON_XCV pXcv;
+
+    TRACE("LocalmonShutdown(%p)\n", hMonitor);
 
     pLocalmon = (PLOCALMON_HANDLE)hMonitor;
 
@@ -77,15 +80,24 @@
         LocalmonClosePort((HANDLE)pPort);
     }
 
-    // Now close all regular ports and remove them from the list.
-    while (!IsListEmpty(&pLocalmon->Ports))
+    // Do the same for the open Xcv ports.
+    while (!IsListEmpty(&pLocalmon->XcvHandles))
     {
-        pPort = CONTAINING_RECORD(&pLocalmon->Ports.Flink, LOCALMON_PORT, Entry);
-        RemoveEntryList(&pPort->Entry);
-        LocalmonClosePort((HANDLE)pPort);
+        pXcv = CONTAINING_RECORD(&pLocalmon->XcvHandles.Flink, LOCALMON_XCV,
Entry);
+        LocalmonXcvClosePort((HANDLE)pXcv);
     }
 
-    // Finally free the memory for the LOCALMON_HANDLE structure itself.
+    // Now close all registry ports, remove them from the list and free their memory.
+    while (!IsListEmpty(&pLocalmon->RegistryPorts))
+    {
+        pPort = CONTAINING_RECORD(&pLocalmon->RegistryPorts.Flink, LOCALMON_PORT,
Entry);
+        LocalmonClosePort((HANDLE)pPort);
+        RemoveEntryList(&pPort->Entry);
+        DllFreeSplMem(pPort);
+    }
+
+    // Finally clean the LOCALMON_HANDLE structure itself.
+    DeleteCriticalSection(&pLocalmon->Section);
     DllFreeSplMem(pLocalmon);
 }
 
@@ -102,10 +114,14 @@
     PLOCALMON_HANDLE pLocalmon;
     PLOCALMON_PORT pPort = NULL;
 
+    TRACE("InitializePrintMonitor2(%p, %p)\n", pMonitorInit, phMonitor);
+
     // Create a new LOCALMON_HANDLE structure.
     pLocalmon = DllAllocSplMem(sizeof(LOCALMON_HANDLE));
+    InitializeCriticalSection(&pLocalmon->Section);
     InitializeListHead(&pLocalmon->FilePorts);
-    InitializeListHead(&pLocalmon->Ports);
+    InitializeListHead(&pLocalmon->RegistryPorts);
+    InitializeListHead(&pLocalmon->XcvHandles);
 
     // The Local Spooler Port Monitor doesn't need to care about the given registry
key and functions.
     // Instead it uses a well-known registry key for getting its information about local
ports. Open this one.
@@ -136,6 +152,7 @@
             goto Cleanup;
         }
 
+        pPort->pLocalmon = pLocalmon;
         pPort->hFile = INVALID_HANDLE_VALUE;
         pPort->pwszPortName = (PWSTR)((PBYTE)pPort + sizeof(LOCALMON_PORT));
 
@@ -156,7 +173,7 @@
         }
 
         // Add it to the list.
-        InsertTailList(&pLocalmon->Ports, &pPort->Entry);
+        InsertTailList(&pLocalmon->RegistryPorts, &pPort->Entry);
 
         // Don't let the cleanup routine free this.
         pPort = NULL;

Modified:
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/ports.c
URL:
http://svn.reactos.org/svn/reactos/branches/colins-printing-for-freedom/rea…
==============================================================================
---
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/ports.c	[iso-8859-1]
(original)
+++
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/ports.c	[iso-8859-1]
Tue Jul  7 10:30:30 2015
@@ -49,7 +49,7 @@
 /**
  * @name _ClosePortHandles
  *
- * Closes a port of any type if its opened.
+ * Closes a port of any type if it's open.
  * Removes any saved mapping or existing definition of a NONSPOOLED device mapping.
  *
  * @param pPort
@@ -305,7 +305,7 @@
     PLIST_ENTRY pEntry;
     PLOCALMON_PORT pPort;
 
-    for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry =
pEntry->Flink)
+    for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry !=
&pLocalmon->RegistryPorts; pEntry = pEntry->Flink)
     {
         pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
 
@@ -329,7 +329,7 @@
     PORT_INFO_1W PortInfo1;
 
     // Count the required buffer size and the number of datatypes.
-    for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry =
pEntry->Flink)
+    for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry !=
&pLocalmon->RegistryPorts; pEntry = pEntry->Flink)
     {
         pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
 
@@ -350,7 +350,7 @@
     pPortString = pPorts + dwPortCount * sizeof(PORT_INFO_1W);
 
     // Copy over all ports.
-    for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry =
pEntry->Flink)
+    for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry !=
&pLocalmon->RegistryPorts; pEntry = pEntry->Flink)
     {
         pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
 
@@ -385,7 +385,7 @@
     PORT_INFO_2W PortInfo2;
 
     // Count the required buffer size and the number of datatypes.
-    for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry =
pEntry->Flink)
+    for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry !=
&pLocalmon->RegistryPorts; pEntry = pEntry->Flink)
     {
         pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
 
@@ -406,7 +406,7 @@
     pPortString = pPorts + dwPortCount * sizeof(PORT_INFO_2W);
 
     // Copy over all ports.
-    for (pEntry = pLocalmon->Ports.Flink; pEntry != &pLocalmon->Ports; pEntry =
pEntry->Flink)
+    for (pEntry = pLocalmon->RegistryPorts.Flink; pEntry !=
&pLocalmon->RegistryPorts; pEntry = pEntry->Flink)
     {
         pPort = CONTAINING_RECORD(pEntry, LOCALMON_PORT, Entry);
 
@@ -472,32 +472,35 @@
 BOOL WINAPI
 LocalmonClosePort(HANDLE hPort)
 {
-    PLOCALMON_PORT pPort;
+    PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
+
+    TRACE("LocalmonClosePort(%p)\n", hPort);
 
     // Sanity checks
-    if (!hPort)
+    if (!pPort)
     {
         SetLastError(ERROR_INVALID_PARAMETER);
         return FALSE;
     }
 
-    pPort = (PLOCALMON_PORT)hPort;
-
-    if (pPort->PortType == PortType_FILE)
-    {
-        // Remove it from the list of virtual file ports.
-        RemoveEntryList(&pPort->Entry);
-    }
-
     // Close the file handle, free memory for pwszMapping and delete any NONSPOOLED
port.
     _ClosePortHandles(pPort);
 
     // Close any open printer handle.
     if (pPort->hPrinter)
+    {
         ClosePrinter(pPort->hPrinter);
-
-    // Finally free the memory for the port itself.
-    DllFreeSplMem(pPort);
+        pPort->hPrinter = NULL;
+    }
+
+    // Free virtual FILE ports which were created in LocalmonOpenPort.
+    if (pPort->PortType == PortType_FILE)
+    {
+        EnterCriticalSection(&pPort->pLocalmon->Section);
+        RemoveEntryList(&pPort->Entry);
+        DllFreeSplMem(pPort);
+        LeaveCriticalSection(&pPort->pLocalmon->Section);
+    }
 
     SetLastError(ERROR_SUCCESS);
     return TRUE;
@@ -507,6 +510,8 @@
 LocalmonEndDocPort(HANDLE hPort)
 {
     PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
+
+    TRACE("LocalmonEndDocPort(%p)\n", hPort);
 
     // Sanity checks
     if (!pPort)
@@ -539,9 +544,12 @@
 LocalmonEnumPorts(HANDLE hMonitor, PWSTR pName, DWORD Level, PBYTE pPorts, DWORD cbBuf,
PDWORD pcbNeeded, PDWORD pcReturned)
 {
     DWORD dwErrorCode;
+    PLOCALMON_HANDLE pLocalmon = (PLOCALMON_HANDLE)hMonitor;
+
+    TRACE("LocalmonEnumPorts(%p, %S, %lu, %p, %lu, %p, %p)\n", hMonitor, pName,
Level, pPorts, cbBuf, pcbNeeded, pcReturned);
 
     // Sanity checks
-    if (!hMonitor || (cbBuf && !pPorts) || !pcbNeeded || !pcReturned)
+    if (!pLocalmon || (cbBuf && !pPorts) || !pcbNeeded || !pcReturned)
     {
         dwErrorCode = ERROR_INVALID_PARAMETER;
         goto Cleanup;
@@ -557,11 +565,15 @@
     *pcbNeeded = 0;
     *pcReturned = 0;
 
+    EnterCriticalSection(&pLocalmon->Section);
+
     // The function behaves differently for each level.
     if (Level == 1)
-        dwErrorCode = _LocalmonEnumPortsLevel1((PLOCALMON_HANDLE)hMonitor, pPorts, cbBuf,
pcbNeeded, pcReturned);
+        dwErrorCode = _LocalmonEnumPortsLevel1(pLocalmon, pPorts, cbBuf, pcbNeeded,
pcReturned);
     else if (Level == 2)
-        dwErrorCode = _LocalmonEnumPortsLevel2((PLOCALMON_HANDLE)hMonitor, pPorts, cbBuf,
pcbNeeded, pcReturned);
+        dwErrorCode = _LocalmonEnumPortsLevel2(pLocalmon, pPorts, cbBuf, pcbNeeded,
pcReturned);
+
+    LeaveCriticalSection(&pLocalmon->Section);
 
 Cleanup:
     SetLastError(dwErrorCode);
@@ -569,7 +581,7 @@
 }
 
 /*
- * @name LocalmonSetPortTimeOuts
+ * @name LocalmonGetPrinterDataFromPort
  *
  * Performs a DeviceIoControl call for the given port.
  *
@@ -608,6 +620,8 @@
     DWORD dwErrorCode;
     PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
 
+    TRACE("LocalmonGetPrinterDataFromPort(%p, %lu, %p, %p, %lu, %p, %lu,
%p)\n", hPort, ControlID, pValueName, lpInBuffer, cbInBuffer, lpOutBuffer,
cbOutBuffer, lpcbReturned);
+
     // Sanity checks
     if (!pPort || !ControlID || !lpcbReturned)
     {
@@ -660,6 +674,8 @@
     PLOCALMON_HANDLE pLocalmon = (PLOCALMON_HANDLE)hMonitor;
     PLOCALMON_PORT pPort;
 
+    TRACE("LocalmonOpenPort(%p, %S, %p)\n", hMonitor, pName, pHandle);
+
     // Sanity checks
     if (!pLocalmon || !pName || !pHandle)
     {
@@ -672,18 +688,23 @@
     {
         // For FILE:, we create a virtual port for each request.
         pPort = DllAllocSplMem(sizeof(LOCALMON_PORT));
+        pPort->pLocalmon = pLocalmon;
         pPort->PortType = PortType_FILE;
         pPort->hFile = INVALID_HANDLE_VALUE;
 
         // Add it to the list of file ports.
+        EnterCriticalSection(&pLocalmon->Section);
         InsertTailList(&pLocalmon->FilePorts, &pPort->Entry);
     }
     else
     {
+        EnterCriticalSection(&pLocalmon->Section);
+
         // Check if the port name is valid.
         pPort = _FindPort(pLocalmon, pName);
         if (!pPort)
         {
+            LeaveCriticalSection(&pLocalmon->Section);
             dwErrorCode = ERROR_UNKNOWN_PORT;
             goto Cleanup;
         }
@@ -693,6 +714,9 @@
         // The others are only opened per job in StartDocPort.
         if (_wcsnicmp(pName, L"LPT", 3) == 0)
         {
+            // Treat all ports as other LPT ports until we can definitely say that
it's a physical one.
+            pPort->PortType = PortType_OtherLPT;
+
             // Try to create a NONSPOOLED port and open it.
             if (_CreateNonspooledPort(pPort))
             {
@@ -705,13 +729,13 @@
                 }
                 else
                 {
-                    // This is no physical port, so don't keep its handle open all
the time.
+                    // This is no physical port, so don't keep its handle open.
                     _ClosePortHandles(pPort);
-                    pPort->PortType = PortType_OtherLPT;
                 }
             }
             else if (GetLastError() != ERROR_SUCCESS)
             {
+                LeaveCriticalSection(&pLocalmon->Section);
                 dwErrorCode = GetLastError();
                 goto Cleanup;
             }
@@ -722,6 +746,8 @@
             pPort->PortType = PortType_PhysicalCOM;
         }
     }
+
+    LeaveCriticalSection(&pLocalmon->Section);
 
     // Return our fetched LOCALMON_PORT structure in the handle.
     *pHandle = (PHANDLE)pPort;
@@ -757,6 +783,8 @@
     DWORD dwErrorCode;
     PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
 
+    TRACE("LocalmonSetPortTimeOuts(%p, %p, %lu)\n", hPort, lpCTO, Reserved);
+
     // Sanity checks
     if (!pPort || !lpCTO)
     {
@@ -809,6 +837,8 @@
     DWORD dwErrorCode;
     PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
 
+    TRACE("LocalmonReadPort(%p, %p, %lu, %p)\n", hPort, pBuffer, cbBuffer,
pcbRead);
+
     // Sanity checks
     if (!pPort || (cbBuffer && !pBuffer) || !pcbRead)
     {
@@ -859,6 +889,8 @@
     DWORD dwErrorCode;
     PDOC_INFO_1W pDocInfo1 = (PDOC_INFO_1W)pDocInfo;        // DOC_INFO_1W is the least
common denominator for both DOC_INFO levels.
     PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
+
+    TRACE("LocalmonStartDocPort(%p, %S, %lu, %lu, %p)\n", hPort, pPrinterName,
JobId, Level, pDocInfo);
 
     // Sanity checks
     if (!pPort || !pPrinterName || (pPort->PortType == PortType_FILE &&
(!pDocInfo1 || !pDocInfo1->pOutputFile || !*pDocInfo1->pOutputFile)))
@@ -954,6 +986,8 @@
     DWORD dwErrorCode;
     PLOCALMON_PORT pPort = (PLOCALMON_PORT)hPort;
 
+    TRACE("LocalmonWritePort(%p, %p, %lu, %p)\n", hPort, pBuffer, cbBuf,
pcbWritten);
+
     // Sanity checks
     if (!pPort || (cbBuf && !pBuffer) || !pcbWritten)
     {

Modified:
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/precomp.h
URL:
http://svn.reactos.org/svn/reactos/branches/colins-printing-for-freedom/rea…
==============================================================================
---
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/precomp.h	[iso-8859-1]
(original)
+++
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/precomp.h	[iso-8859-1]
Tue Jul  7 10:30:30 2015
@@ -29,6 +29,19 @@
 
 // Structures
 /**
+ * Describes the monitor handle returned by InitializePrintMonitor2.
+ * Manages all available ports in this instance.
+ */
+typedef struct _LOCALMON_HANDLE
+{
+    CRITICAL_SECTION Section;       /** Critical Section for modifying or reading the
ports. */
+    LIST_ENTRY FilePorts;           /** Virtual ports created for every document
that's redirected to an output file. */
+    LIST_ENTRY RegistryPorts;       /** COM, FILE: and LPT ports loaded from the local
registry. */
+    LIST_ENTRY XcvHandles;          /** Xcv handles created with LocalmonXcvOpenPort. */
+}
+LOCALMON_HANDLE, *PLOCALMON_HANDLE;
+
+/**
  * Describes the port handle returned by LocalmonOpenPort.
  * Manages a legacy port (COM/LPT) or virtual FILE: port for printing as well as its
associated printer and job.
  */
@@ -46,21 +59,11 @@
     DWORD dwJobID;                  /** ID of the printing job we are processing (for
later reporting progress using SetJobW). */
     HANDLE hFile;                   /** Handle to the opened port or INVALID_HANDLE_VALUE
if it isn't currently opened. */
     HANDLE hPrinter;                /** Handle to the printer for the job on this port
(for using SetJobW). */
+    PLOCALMON_HANDLE pLocalmon;     /** Pointer to the parent LOCALMON_HANDLE structure.
*/
     PWSTR pwszMapping;              /** The current mapping of the DOS Device
corresponding to this port at the time _CreateNonspooledPort has been called. */
     PWSTR pwszPortName;             /** The name of this port including the trailing
colon. Empty for virtual file ports. */
 }
 LOCALMON_PORT, *PLOCALMON_PORT;
-
-/**
- * Describes the monitor handle returned by InitializePrintMonitor2.
- * Manages all available ports in this instance.
- */
-typedef struct _LOCALMON_HANDLE
-{
-    LIST_ENTRY FilePorts;           /** Virtual ports created for every document
that's redirected to an output file. */
-    LIST_ENTRY Ports;               /** Ports found on the system (except for FILE:) */
-}
-LOCALMON_HANDLE, *PLOCALMON_HANDLE;
 
 /**
  * Describes the Xcv handle returned by LocalmonXcvOpenPort.
@@ -68,7 +71,9 @@
  */
 typedef struct _LOCALMON_XCV
 {
+    LIST_ENTRY Entry;
     ACCESS_MASK GrantedAccess;
+    PLOCALMON_HANDLE pLocalmon;
     PWSTR pwszObject;
 }
 LOCALMON_XCV, *PLOCALMON_XCV;

Modified:
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/xcv.c
URL:
http://svn.reactos.org/svn/reactos/branches/colins-printing-for-freedom/rea…
==============================================================================
---
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/xcv.c	[iso-8859-1]
(original)
+++
branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/xcv.c	[iso-8859-1]
Tue Jul  7 10:30:30 2015
@@ -345,15 +345,21 @@
 BOOL WINAPI
 LocalmonXcvClosePort(HANDLE hXcv)
 {
-    // Sanity checks
-    if (!hXcv)
+    PLOCALMON_XCV pXcv = (PLOCALMON_XCV)hXcv;
+
+    TRACE("LocalmonXcvClosePort(%p)\n", hXcv);
+
+    // Sanity checks
+    if (!pXcv)
     {
         SetLastError(ERROR_INVALID_PARAMETER);
         return FALSE;
     }
 
-    // Free the memory.
-    DllFreeSplMem(hXcv);
+    // Remove it from the list and free the memory.
+    RemoveEntryList(&pXcv->Entry);
+    DllFreeSplMem(pXcv);
+
     SetLastError(ERROR_SUCCESS);
     return TRUE;
 }
@@ -361,6 +367,8 @@
 DWORD WINAPI
 LocalmonXcvDataPort(HANDLE hXcv, PCWSTR pszDataName, PBYTE pInputData, DWORD cbInputData,
PBYTE pOutputData, DWORD cbOutputData, PDWORD pcbOutputNeeded)
 {
+    TRACE("LocalmonXcvDataPort(%p, %S, %p, %lu, %p, %lu, %p)\n", hXcv,
pszDataName, pInputData, cbInputData, pOutputData, cbOutputData, pcbOutputNeeded);
+
     // Sanity checks
     if (!pszDataName)
         return ERROR_INVALID_PARAMETER;
@@ -401,10 +409,13 @@
 {
     DWORD cbObject = 0;
     DWORD dwErrorCode;
+    PLOCALMON_HANDLE pLocalmon = (PLOCALMON_HANDLE)hMonitor;
     PLOCALMON_XCV pXcv;
 
-    // Sanity checks
-    if (!hMonitor || !phXcv)
+    TRACE("LocalmonXcvOpenPort(%p, %S, %lu, %p)\n", hMonitor, pwszObject,
GrantedAccess, phXcv);
+
+    // Sanity checks
+    if (!pLocalmon || !phXcv)
     {
         dwErrorCode = ERROR_INVALID_PARAMETER;
         goto Cleanup;
@@ -422,6 +433,7 @@
         goto Cleanup;
     }
 
+    pXcv->pLocalmon = pLocalmon;
     pXcv->GrantedAccess = GrantedAccess;
 
     if (cbObject)
@@ -429,6 +441,8 @@
         pXcv->pwszObject = (PWSTR)((PBYTE)pXcv + sizeof(LOCALMON_XCV));
         CopyMemory(pXcv->pwszObject, pwszObject, cbObject);
     }
+
+    InsertTailList(&pLocalmon->XcvHandles, &pXcv->Entry);
 
     // Return it as the Xcv handle.
     *phXcv = (HANDLE)pXcv;