[cfinck] 68402: [LOCALMON] My idea to just care about COM, FILE: and LPT ports was too short-sighted. Apart from selecting a FILE: port that prompts for the output filename at printing, you can als...

Author: cfinck
Date: Wed Jul 15 18:15:33 2015
New Revision: 68402

URL: http://svn.reactos.org/svn/reactos?rev=68402&view=rev
Log:
[LOCALMON]
My idea to just care about COM, FILE: and LPT ports was too short-sighted.
Apart from selecting a FILE: port that prompts for the output filename at printing, you can also add a port "C:\bla.txt" to always output into that particular file.
Even shared network printers can be added as a local port "\COMPUTERNAME\PrinterName" (and Windows even does that when auto-adding printers found on the network). Note that this is the exception though, shared network printers are normally handled by a different component.

Our localmon now handles all valid ports found in the registry.
Port name checks were modified to be extra-picky and not let any false positives happen (e.g. trying to print into a file starting with "LPT" shouldn't be treated as printing to an LPT port)

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/tools.c
    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/reac...
==============================================================================
--- 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] Wed Jul 15 18:15:33 2015
@@ -38,6 +38,52 @@
 };

+/**
+ * @name _IsNEPort
+ *
+ * Checks if the given port name is a virtual Ne port.
+ * A virtual Ne port may appear in HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\Windows NT\CurrentVersion\Ports and can have the formats
+ * Ne00:, Ne01:, Ne-02:, Ne456:
+ * This check is extra picky to not cause false positives (like file name ports starting with "Ne").
+ *
+ * @param pwszPortName
+ * The port name to check.
+ *
+ * @return
+ * TRUE if this is definitely a virtual Ne port, FALSE if not.
+ */
+static __inline BOOL
+_IsNEPort(PCWSTR pwszPortName)
+{
+    PCWSTR p = pwszPortName;
+
+    // First character needs to be 'N' (uppercase or lowercase)
+    if (*p != L'N' && *p != L'n')
+        return FALSE;
+
+    // Next character needs to be 'E' (uppercase or lowercase)
+    p++;
+    if (*p != L'E' && *p != L'e')
+        return FALSE;
+
+    // An optional hyphen may follow now.
+    p++;
+    if (*p == L'-')
+        p++;
+
+    // Now an arbitrary number of digits may follow.
+    while (*p >= L'0' && *p <= L'9')
+        p++;
+
+    // Finally, the virtual Ne port must be terminated by a colon.
+    if (*p != ':')
+        return FALSE;
+
+    // If this is the end of the string, we have a virtual Ne port.
+    p++;
+    return (*p == L'\0');
+}
+
 static void
 _LoadResources(HINSTANCE hinstDLL)
 {
@@ -165,8 +211,16 @@
             goto Cleanup;
         }

-        // This Port Monitor supports COM, FILE and LPT ports. Skip all others.
-        if (_wcsnicmp(pPort->pwszPortName, L"COM", 3) != 0 && _wcsicmp(pPort->pwszPortName, L"FILE:") != 0 && _wcsnicmp(pPort->pwszPortName, L"LPT", 3) != 0)
+        // pwszPortName can be one of the following to be valid for this Port Monitor:
+        //    COMx:                        - Physical COM port
+        //    LPTx:                        - Physical LPT port (or redirected one using "net use LPT1 ...")
+        //    FILE:                        - Opens a prompt that asks for an output filename
+        //    C:\bla.txt                   - Redirection into the file "C:\bla.txt"
+        //    \COMPUTERNAME\PrinterName   - Redirection to a shared network printer installed as a local port
+        //
+        // We can't detect valid and invalid ones by the name, so we can only exclude empty ports and the virtual "Ne00:", "Ne01:", ... ports.
+        // Skip the invalid ones here.
+        if (!cchPortName || _IsNEPort(pPort->pwszPortName))
         {
             DllFreeSplMem(pPort);
             continue;

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/reac...
==============================================================================
--- 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] Wed Jul 15 18:15:33 2015
@@ -17,33 +17,76 @@
  * @name _GetNonspooledPortName
  *
  * Prepends "NONSPOOLED_" to a port name without colon.
- * You have to free the returned buffer using DllFreeSplMem.
  *
  * @param pwszPortNameWithoutColon
  * Result of a previous GetPortNameWithoutColon call.
  *
+ * @param ppwszNonspooledPortName
+ * Pointer to a buffer that will contain the NONSPOOLED port name.
+ * You have to free this buffer using DllFreeSplMem.
+ *
  * @return
- * Buffer containing the NONSPOOLED port name or NULL in case of failure.
+ * ERROR_SUCCESS if the NONSPOOLED port name was successfully copied into the buffer.
+ * ERROR_NOT_ENOUGH_MEMORY if memory allocation failed.
  */
-static __inline PWSTR
-_GetNonspooledPortName(PCWSTR pwszPortNameWithoutColon)
+static __inline DWORD
+_GetNonspooledPortName(PCWSTR pwszPortNameWithoutColon, PWSTR* ppwszNonspooledPortName)
 {
     DWORD cchPortNameWithoutColon;
-    PWSTR pwszNonspooledPortName;

cchPortNameWithoutColon = wcslen(pwszPortNameWithoutColon);

-    pwszNonspooledPortName = DllAllocSplMem((cchNonspooledPrefix + cchPortNameWithoutColon + 1) * sizeof(WCHAR));
-    if (!pwszNonspooledPortName)
+    *ppwszNonspooledPortName = DllAllocSplMem((cchNonspooledPrefix + cchPortNameWithoutColon + 1) * sizeof(WCHAR));
+    if (!*ppwszNonspooledPortName)
     {
         ERR("DllAllocSplMem failed with error %lu!\n", GetLastError());
-        return NULL;
-    }
-
-    CopyMemory(pwszNonspooledPortName, wszNonspooledPrefix, cchNonspooledPrefix * sizeof(WCHAR));
-    CopyMemory(&pwszNonspooledPortName[cchNonspooledPrefix], pwszPortNameWithoutColon, (cchPortNameWithoutColon + 1) * sizeof(WCHAR));
-
-    return pwszNonspooledPortName;
+        return ERROR_NOT_ENOUGH_MEMORY;
+    }
+
+    CopyMemory(*ppwszNonspooledPortName, wszNonspooledPrefix, cchNonspooledPrefix * sizeof(WCHAR));
+    CopyMemory(&(*ppwszNonspooledPortName)[cchNonspooledPrefix], pwszPortNameWithoutColon, (cchPortNameWithoutColon + 1) * sizeof(WCHAR));
+
+    return ERROR_SUCCESS;
+}
+
+/**
+ * @name _IsLegacyPort
+ *
+ * Checks if the given port name is a legacy port (COM or LPT).
+ * This check is extra picky to not cause false positives (like file name ports starting with "COM" or "LPT").
+ *
+ * @param pwszPortName
+ * The port name to check.
+ *
+ * @param pwszPortType
+ * L"COM" or L"LPT"
+ *
+ * @return
+ * TRUE if this is definitely the asked legacy port, FALSE if not.
+ */
+static __inline BOOL
+_IsLegacyPort(PCWSTR pwszPortName, PCWSTR pwszPortType)
+{
+    const DWORD cchPortType = 3;
+    PCWSTR p = pwszPortName;
+
+    // The port name must begin with pwszPortType.
+    if (_wcsnicmp(p, pwszPortType, cchPortType) != 0)
+        return FALSE;
+
+    p += cchPortType;
+
+    // Now an arbitrary number of digits may follow.
+    while (*p >= L'0' && *p <= L'9')
+        p++;
+
+    // Finally, the legacy port must be terminated by a colon.
+    if (*p != ':')
+        return FALSE;
+
+    // If this is the end of the string, we have a legacy port.
+    p++;
+    return (*p == L'\0');
 }

/**
@@ -78,11 +121,9 @@
     pPort->pwszMapping = NULL;

// Finally get the required strings and remove the DOS device definition for the NONSPOOLED port.
-    pwszPortNameWithoutColon = GetPortNameWithoutColon(pPort->pwszPortName);
-    if (pwszPortNameWithoutColon)
-    {
-        pwszNonspooledPortName = _GetNonspooledPortName(pwszPortNameWithoutColon);
-        if (pwszNonspooledPortName)
+    if (GetPortNameWithoutColon(pPort->pwszPortName, &pwszPortNameWithoutColon) == ERROR_SUCCESS)
+    {
+        if (_GetNonspooledPortName(pwszPortNameWithoutColon, &pwszNonspooledPortName) == ERROR_SUCCESS)
         {
             DefineDosDeviceW(DDD_REMOVE_DEFINITION, pwszNonspooledPortName, NULL);
             DllFreeSplMem(pwszNonspooledPortName);
@@ -105,8 +146,7 @@
  * @return
  * TRUE if a NONSPOOLED port was successfully created, FALSE otherwise.
  * A more specific error code can be obtained through GetLastError.
- * In particular, if the return value is FALSE and GetLastError returns ERROR_SUCCESS, no NONSPOOLED port is needed,
- * because no system-wide device definition is available.
+ * In particular, if the return value is FALSE and GetLastError returns ERROR_SUCCESS, no NONSPOOLED port is needed for this port.
  */
 static BOOL
 _CreateNonspooledPort(PLOCALMON_PORT pPort)
@@ -129,10 +169,16 @@
     PWSTR pwszPortNameWithoutColon = NULL;

// We need the port name without the trailing colon.
-    pwszPortNameWithoutColon = GetPortNameWithoutColon(pPort->pwszPortName);
-    if (!pwszPortNameWithoutColon)
-    {
-        dwErrorCode = ERROR_NOT_ENOUGH_MEMORY;
+    dwErrorCode = GetPortNameWithoutColon(pPort->pwszPortName, &pwszPortNameWithoutColon);
+    if (dwErrorCode == ERROR_INVALID_PARAMETER)
+    {
+        // This port has no trailing colon, so we also need no NONSPOOLED mapping for it.
+        dwErrorCode = ERROR_SUCCESS;
+        goto Cleanup;
+    }
+    else if (dwErrorCode != ERROR_SUCCESS)
+    {
+        // Another unexpected failure.
         goto Cleanup;
     }

@@ -222,12 +268,9 @@
     }

// We now want to create a DOS device "NONSPOOLED_<PortName>" to this mapping, so that we're able to open it through CreateFileW.
-    pwszNonspooledPortName = _GetNonspooledPortName(pwszPortNameWithoutColon);
-    if (!pwszNonspooledPortName)
-    {
-        dwErrorCode = ERROR_NOT_ENOUGH_MEMORY;
-        goto Cleanup;
-    }
+    dwErrorCode = _GetNonspooledPortName(pwszPortNameWithoutColon, &pwszNonspooledPortName);
+    if (dwErrorCode != ERROR_SUCCESS)
+        goto Cleanup;

// Delete a possibly existing NONSPOOLED device before creating the new one, so we don't stack up device definitions.
     DefineDosDeviceW(DDD_REMOVE_DEFINITION, pwszNonspooledPortName, NULL);
@@ -493,7 +536,7 @@
         pPort->hPrinter = NULL;
     }

-    // Free virtual FILE ports which were created in LocalmonOpenPort.
+    // Free virtual FILE: ports which were created in LocalmonOpenPort.
     if (pPort->PortType == PortType_FILE)
     {
         EnterCriticalSection(&pPort->pLocalmon->Section);
@@ -712,11 +755,8 @@
         // Even if this API is called OpenPort, port file handles aren't always opened here :-P
         // Windows only does this for physical LPT ports here to enable bidirectional communication with the printer outside of jobs (using ReadPort and WritePort).
         // 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;
-
+        if (_IsLegacyPort(pName, L"LPT"))
+        {
             // Try to create a NONSPOOLED port and open it.
             if (_CreateNonspooledPort(pPort))
             {
@@ -740,9 +780,9 @@
                 goto Cleanup;
             }
         }
-        else
-        {
-            // This can only be a COM port.
+        else if (_IsLegacyPort(pName, L"COM"))
+        {
+            // COM ports can't be redirected over the network, so this is a physical one.
             pPort->PortType = PortType_PhysicalCOM;
         }
     }
@@ -948,13 +988,17 @@
     }
     else
     {
-        // This is a COM port or a non-physical LPT port. We open NONSPOOLED ports for these per job.
+        // This can be:
+        //    - a physical COM port
+        //    - a non-physical LPT port (e.g. with "net use LPT1 ...")
+        //    - any other port (e.g. a file or a shared printer installed as a local port)
+        //
+        // For all these cases, we try to create a NONSPOOLED port per job.
+        // If _CreateNonspooledPort reports that no NONSPOOLED port is necessary, we can just open the port name.
         if (!_CreateNonspooledPort(pPort))
         {
             if (GetLastError() == ERROR_SUCCESS)
             {
-                // This is a user-local instead of a system-wide port.
-                // Such local ports haven't been remapped by the spooler, so we can just open them.
                 pPort->hFile = CreateFileW(pPort->pwszPortName, GENERIC_WRITE, FILE_SHARE_READ, NULL, OPEN_ALWAYS, 0, NULL);
                 if (pPort->hFile == INVALID_HANDLE_VALUE)
                 {

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/reac...
==============================================================================
--- 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] Wed Jul 15 18:15:33 2015
@@ -35,8 +35,8 @@
 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 FilePorts;           /** Ports created when a document is printed on FILE: and the user entered a file name. */
+    LIST_ENTRY RegistryPorts;       /** Valid ports loaded from the local registry. */
     LIST_ENTRY XcvHandles;          /** Xcv handles created with LocalmonXcvOpenPort. */
 }
 LOCALMON_HANDLE, *PLOCALMON_HANDLE;
@@ -49,10 +49,10 @@
 {
     LIST_ENTRY Entry;
     enum {
-        PortType_FILE,              /** A virtual port for redirecting the document into a file. */
+        PortType_Other = 0,         /** Any port that doesn't belong into the other categories (default). */
+        PortType_FILE,              /** A port created when a document is printed on FILE: and the user entered a file name. */
         PortType_PhysicalCOM,       /** A physical serial port (COM) */
-        PortType_PhysicalLPT,       /** A physical parallel port (LPT) */
-        PortType_OtherLPT           /** A non-physical parallel port (e.g. a redirected one over network using "net use LPT1 ...") */
+        PortType_PhysicalLPT        /** A physical parallel port (LPT) */
     }
     PortType;
     BOOL bStartedDoc;               /** Whether a document has been started with StartDocPort. */
@@ -99,7 +99,7 @@
 // tools.c
 BOOL DoesPortExist(PCWSTR pwszPortName);
 DWORD GetLPTTransmissionRetryTimeout();
-PWSTR GetPortNameWithoutColon(PCWSTR pwszPortName);
+DWORD GetPortNameWithoutColon(PCWSTR pwszPortName, PWSTR* ppwszPortNameWithoutColon);

// xcv.c
 BOOL WINAPI LocalmonXcvClosePort(HANDLE hXcv);

Modified: branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/tools.c
URL: http://svn.reactos.org/svn/reactos/branches/colins-printing-for-freedom/reac...
==============================================================================
--- branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/tools.c	[iso-8859-1] (original)
+++ branches/colins-printing-for-freedom/reactos/win32ss/printing/monitors/localmon/tools.c	[iso-8859-1] Wed Jul 15 18:15:33 2015
@@ -124,32 +124,42 @@
  * @name GetPortNameWithoutColon
  *
  * Most of the time, we operate on port names with a trailing colon. But some functions require the name without the trailing colon.
- * This function returns the port name without the colon. You have to free the returned buffer using DllFreeSplMem.
+ * This function checks if the port has a trailing colon and if so, it returns the port name without the colon.
  *
  * @param pwszPortName
  * The port name with colon
  *
+ * @param ppwszPortNameWithoutColon
+ * Pointer to a PWSTR that will contain the port name without colon.
+ * You have to free this buffer using DllFreeSplMem.
+ *
  * @return
- * Buffer containing the port name without a colon or NULL in case of failure.
+ * ERROR_SUCCESS if the port name without colon was successfully copied into the buffer.
+ * ERROR_INVALID_PARAMETER if this port name has no trailing colon.
+ * ERROR_NOT_ENOUGH_MEMORY if memory allocation failed.
  */
-PWSTR
-GetPortNameWithoutColon(PCWSTR pwszPortName)
+DWORD
+GetPortNameWithoutColon(PCWSTR pwszPortName, PWSTR* ppwszPortNameWithoutColon)
 {
     DWORD cchPortName;
-    PWSTR pwszPortNameWithoutColon;

-    // Every port in our port list has a trailing colon, so we just need to remove the last character of the string.
+    // Compute the string length of pwszPortNameWithoutColon.
     cchPortName = wcslen(pwszPortName) - 1;

-    pwszPortNameWithoutColon = DllAllocSplMem((cchPortName + 1) * sizeof(WCHAR));
-    if (!pwszPortNameWithoutColon)
+    // Check if pwszPortName really has a colon as the last character.
+    if (pwszPortName[cchPortName] != L':')
+        return ERROR_INVALID_PARAMETER;
+
+    // It has, so allocate a buffer and copy the port name without colon into it.
+    *ppwszPortNameWithoutColon = DllAllocSplMem((cchPortName + 1) * sizeof(WCHAR));
+    if (!*ppwszPortNameWithoutColon)
     {
         ERR("DllAllocSplMem failed with error %lu!\n", GetLastError());
-        return NULL;
+        return ERROR_NOT_ENOUGH_MEMORY;
     }

-    CopyMemory(pwszPortNameWithoutColon, pwszPortName, cchPortName * sizeof(WCHAR));
-    pwszPortNameWithoutColon[cchPortName] = 0;
+    CopyMemory(*ppwszPortNameWithoutColon, pwszPortName, cchPortName * sizeof(WCHAR));
+    *ppwszPortNameWithoutColon[cchPortName] = 0;

-    return pwszPortNameWithoutColon;
+    return ERROR_SUCCESS;
 }

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/reac...
==============================================================================
--- 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] Wed Jul 15 18:15:33 2015
@@ -305,12 +305,9 @@
     }

// SetDefaultCommConfigW needs the port name without colon.
-    pwszPortNameWithoutColon = GetPortNameWithoutColon(pXcv->pwszObject);
-    if (!pwszPortNameWithoutColon)
-    {
-        dwErrorCode = ERROR_NOT_ENOUGH_MEMORY;
-        goto Cleanup;
-    }
+    dwErrorCode = GetPortNameWithoutColon(pXcv->pwszObject, &pwszPortNameWithoutColon);
+    if (dwErrorCode != ERROR_SUCCESS)
+        goto Cleanup;

// Switch to the SYSTEM context for setting the port configuration.
     hToken = RevertToPrinterSelf();