[PATCH v2 3/4] localspl: Add support for monitors providing the MONITOR2 interface.

[Dmitry Timoshkov]

Signed-off-by: Dmitry Timoshkov 
---
 dlls/localspl/provider.c | 144 ++++++++++++++++++++++++++-------------
 1 file changed, 96 insertions(+), 48 deletions(-)

diff --git a/dlls/localspl/provider.c b/dlls/localspl/provider.c
index 8e5e3428d5..44cf38e571 100644
--- a/dlls/localspl/provider.c
+++ b/dlls/localspl/provider.c
@@ -65,10 +65,11 @@ typedef struct {
     LPWSTR          name;
     LPWSTR          dllname;
     PMONITORUI      monitorUI;
-    LPMONITOR       monitor;
+    MONITOR2        monitor;
+    BOOL  (WINAPI *old_XcvOpenPort)(LPCWSTR,ACCESS_MASK,PHANDLE);
+    DWORD (WINAPI *old_XcvDataPort)(HANDLE,LPCWSTR,PBYTE,DWORD,PBYTE,DWORD,PDWORD);
     HMODULE         hdll;
     DWORD           refcount;
-    DWORD           dwMonitorSize;
 } monitor_t;

typedef struct {
@@ -327,6 +328,10 @@ static void monitor_unload(monitor_t * pm)

if (pm->refcount == 0) {
         list_remove(&pm->entry);
+
+        if (pm->monitor.pfnShutdown)
+            pm->monitor.pfnShutdown(0);
+
         FreeLibrary(pm->hdll);
         heap_free(pm->name);
         heap_free(pm->dllname);
@@ -352,7 +357,7 @@ static void monitor_unloadall(void)
     LIST_FOR_EACH_ENTRY_SAFE(pm, next, &monitor_handles, monitor_t, entry)
     {
         /* skip monitorui dlls */
-        if (pm->monitor) monitor_unload(pm);
+        if (pm->monitor.cbSize) monitor_unload(pm);
     }
     LeaveCriticalSection(&monitor_handles_cs);
 }
@@ -378,6 +383,7 @@ static monitor_t * monitor_load(LPCWSTR name, LPWSTR dllname)
     monitor_t * cursor;
     LPWSTR  regroot = NULL;
     LPWSTR  driver = dllname;
+    HKEY    hroot = 0;

TRACE("(%s, %s)\n", debugstr_w(name), debugstr_w(dllname));
     /* Is the Monitor already loaded? */
@@ -402,7 +408,6 @@ static monitor_t * monitor_load(LPCWSTR name, LPWSTR dllname)

if (pm->name == NULL) {
         /* Load the monitor */
-        LPMONITOREX pmonitorEx;
         DWORD   len;

if (name) {
@@ -413,19 +418,19 @@ static monitor_t * monitor_load(LPCWSTR name, LPWSTR dllname)
         if (regroot) {
             lstrcpyW(regroot, monitorsW);
             lstrcatW(regroot, name);
-            /* Get the Driver from the Registry */
-            if (driver == NULL) {
-                HKEY    hroot;
-                DWORD   namesize;
-                if (RegOpenKeyW(HKEY_LOCAL_MACHINE, regroot, &hroot) == ERROR_SUCCESS) {
+            if (RegOpenKeyW(HKEY_LOCAL_MACHINE, regroot, &hroot) == ERROR_SUCCESS) {
+                /* Get the Driver from the Registry */
+                if (driver == NULL) {
+                    DWORD   namesize;
                     if (RegQueryValueExW(hroot, driverW, NULL, NULL, NULL,
                                         &namesize) == ERROR_SUCCESS) {
                         driver = heap_alloc(namesize);
                         RegQueryValueExW(hroot, driverW, NULL, NULL, (LPBYTE) driver, &namesize) ;
                     }
-                    RegCloseKey(hroot);
                 }
             }
+            else
+                WARN("%s not found\n", debugstr_w(regroot));
         }

pm->name = strdupW(name);
@@ -471,26 +476,40 @@ static monitor_t * monitor_load(LPCWSTR name, LPWSTR dllname)
             }
         }

-        if (pInitializePrintMonitor && regroot) {
+        if (pInitializePrintMonitor2 && hroot) {
+            MONITORINIT init;
+            MONITOR2 *monitor2;
+            HANDLE hmon;
+
+            memset(&init, 0, sizeof(init));
+            init.cbSize = sizeof(init);
+            init.hckRegistryRoot = hroot;
+            init.bLocal = TRUE;
+
+            monitor2 = pInitializePrintMonitor2(&init, &hmon);
+            TRACE("%p: MONITOR2 from %s,InitializePrintMonitor2(%s)\n",
+                    monitor2, debugstr_w(driver), debugstr_w(regroot));
+            if (monitor2)
+                memcpy(&pm->monitor, monitor2, min(monitor2->cbSize, sizeof(pm->monitor)));
+        }
+        else if (pInitializePrintMonitor && regroot) {
+            MONITOREX *pmonitorEx;
+
             pmonitorEx = pInitializePrintMonitor(regroot);
             TRACE("%p: LPMONITOREX from %s,InitializePrintMonitor(%s)\n",
                     pmonitorEx, debugstr_w(driver), debugstr_w(regroot));
-
-            if (pmonitorEx) {
-                pm->dwMonitorSize = pmonitorEx->dwMonitorSize;
-                pm->monitor = &(pmonitorEx->Monitor);
+            if (pmonitorEx)
+            {
+                /* Layout of MONITOREX and MONITOR2 mostly matches */
+                memcpy(&pm->monitor, pmonitorEx, min(pmonitorEx->dwMonitorSize, sizeof(pm->monitor)));
+                pm->old_XcvOpenPort = pmonitorEx->Monitor.pfnXcvOpenPort;
+                pm->old_XcvDataPort = pmonitorEx->Monitor.pfnXcvDataPort;
+                pm->monitor.pfnXcvOpenPort = NULL;
+                pm->monitor.pfnXcvDataPort = NULL;
             }
         }

-        if (pm->monitor) {
-            TRACE("0x%08x: dwMonitorSize (%d)\n", pm->dwMonitorSize, pm->dwMonitorSize);
-
-        }
-
-        if (!pm->monitor && regroot) {
-            if (pInitializePrintMonitor2 != NULL) {
-                FIXME("%s,InitializePrintMonitor2 not implemented\n", debugstr_w(driver));
-            }
+        if (!pm->monitor.cbSize && regroot) {
             if (pInitializeMonitorEx != NULL) {
                 FIXME("%s,InitializeMonitorEx not implemented\n", debugstr_w(driver));
             }
@@ -498,7 +517,7 @@ static monitor_t * monitor_load(LPCWSTR name, LPWSTR dllname)
                 FIXME("%s,InitializeMonitor not implemented\n", debugstr_w(driver));
             }
         }
-        if (!pm->monitor && !pm->monitorUI) {
+        if (!pm->monitor.cbSize && !pm->monitorUI) {
             monitor_unload(pm);
             SetLastError(ERROR_PROC_NOT_FOUND);
             pm = NULL;
@@ -511,6 +530,7 @@ cleanup:
     }
     LeaveCriticalSection(&monitor_handles_cs);
     if (driver != dllname) heap_free(driver);
+    if (hroot) RegCloseKey(hroot);
     heap_free(regroot);
     TRACE("=> %p\n", pm);
     return pm;
@@ -577,15 +597,26 @@ static monitor_t * monitor_loadui(monitor_t * pm)
     }

/* query the userinterface-dllname from the Portmonitor */
-    if ((pm->monitor) && (pm->monitor->pfnXcvDataPort)) {
+    if (pm->monitor.pfnXcvDataPort) {
+        res = pm->monitor.pfnXcvOpenPort(0, emptyW, SERVER_ACCESS_ADMINISTER, &hXcv);
+        TRACE("got %u with %p\n", res, hXcv);
+        if (res) {
+            res = pm->monitor.pfnXcvDataPort(hXcv, monitorUIW, NULL, 0, (BYTE *) buffer, sizeof(buffer), &len);
+            TRACE("got %u with %s\n", res, debugstr_w(buffer));
+            if (res == ERROR_SUCCESS) pui = monitor_load(NULL, buffer);
+            pm->monitor.pfnXcvClosePort(hXcv);
+        }
+        return pui;
+    }
+    else if (pm->old_XcvDataPort) {
         /* building (",XcvMonitor %s",pm->name) not needed yet */
-        res = pm->monitor->pfnXcvOpenPort(emptyW, SERVER_ACCESS_ADMINISTER, &hXcv);
+        res = pm->old_XcvOpenPort(emptyW, SERVER_ACCESS_ADMINISTER, &hXcv);
         TRACE("got %u with %p\n", res, hXcv);
         if (res) {
-            res = pm->monitor->pfnXcvDataPort(hXcv, monitorUIW, NULL, 0, (BYTE *) buffer, sizeof(buffer), &len);
+            res = pm->monitor.pfnXcvDataPort(hXcv, monitorUIW, NULL, 0, (BYTE *) buffer, sizeof(buffer), &len);
             TRACE("got %u with %s\n", res, debugstr_w(buffer));
             if (res == ERROR_SUCCESS) pui = monitor_load(NULL, buffer);
-            pm->monitor->pfnXcvClosePort(hXcv);
+            pm->monitor.pfnXcvClosePort(hXcv);
         }
     }
     return pui;
@@ -921,7 +952,6 @@ static DWORD get_ports_from_all_monitors(DWORD level, LPBYTE pPorts, DWORD cbBuf
     DWORD   numentries;
     DWORD   entrysize;

-
     TRACE("(%d, %p, %d, %p)\n", level, pPorts, cbBuf, lpreturned);
     entrysize = (level == 1) ? sizeof(PORT_INFO_1W) : sizeof(PORT_INFO_2W);

@@ -934,16 +964,16 @@ static DWORD get_ports_from_all_monitors(DWORD level, LPBYTE pPorts, DWORD cbBuf

LIST_FOR_EACH_ENTRY(pm, &monitor_handles, monitor_t, entry)
     {
-        if ((pm->monitor) && (pm->monitor->pfnEnumPorts)) {
+        if (pm->monitor.pfnEnumPorts) {
             pi_needed = 0;
             pi_returned = 0;
-            res = pm->monitor->pfnEnumPorts(NULL, level, pi_buffer, pi_allocated, &pi_needed, &pi_returned);
+            res = pm->monitor.pfnEnumPorts(NULL, level, pi_buffer, pi_allocated, &pi_needed, &pi_returned);
             if (!res && (GetLastError() == ERROR_INSUFFICIENT_BUFFER)) {
                 /* Do not use heap_realloc (we do not need the old data in the buffer) */
                 heap_free(pi_buffer);
                 pi_buffer = heap_alloc(pi_needed);
                 pi_allocated = (pi_buffer) ? pi_needed : 0;
-                res = pm->monitor->pfnEnumPorts(NULL, level, pi_buffer, pi_allocated, &pi_needed, &pi_returned);
+                res = pm->monitor.pfnEnumPorts(NULL, level, pi_buffer, pi_allocated, &pi_needed, &pi_returned);
             }
             TRACE("(%s) got %d with %d (need %d byte for %d entries)\n",
                   debugstr_w(pm->name), res, GetLastError(), pi_needed, pi_returned);
@@ -1144,7 +1174,10 @@ static HMODULE driver_load(const printenv_t * env, LPWSTR dllname)
 static VOID printer_free(printer_t * printer)
 {
     if (printer->hXcv)
-        printer->pm->monitor->pfnXcvClosePort(printer->hXcv);
+    {
+        if (printer->pm->monitor.pfnXcvClosePort)
+            printer->pm->monitor.pfnXcvClosePort(printer->hXcv);
+    }

monitor_unload(printer->pm);

@@ -1224,11 +1257,14 @@ static HANDLE printer_alloc_handle(LPCWSTR name, LPPRINTER_DEFAULTSW pDefault)
         }

if (printer->pm) {
-            if ((printer->pm->monitor) && (printer->pm->monitor->pfnXcvOpenPort)) {
-                printer->pm->monitor->pfnXcvOpenPort(&printername[len],
-                                                    pDefault ? pDefault->DesiredAccess : 0,
-                                                    &printer->hXcv);
-            }
+            if (printer->pm->monitor.pfnXcvOpenPort)
+                printer->pm->monitor.pfnXcvOpenPort(0, &printername[len],
+                                                   pDefault ? pDefault->DesiredAccess : 0,
+                                                   &printer->hXcv);
+            else if (printer->pm->old_XcvOpenPort)
+                printer->pm->old_XcvOpenPort(&printername[len],
+                                                   pDefault ? pDefault->DesiredAccess : 0,
+                                                   &printer->hXcv);
             if (printer->hXcv == NULL) {
                 printer_free(printer);
                 SetLastError(ERROR_INVALID_PARAMETER);
@@ -1587,8 +1623,10 @@ static BOOL WINAPI fpAddPort(LPWSTR pName, HWND hWnd, LPWSTR pMonitorName)
     }

pm = monitor_load(pMonitorName, NULL);
-    if (pm && pm->monitor && pm->monitor->pfnAddPort) {
-        res = pm->monitor->pfnAddPort(pName, hWnd, pMonitorName);
+    if (!pm) return FALSE;
+
+    if (pm->monitor.pfnAddPort) {
+        res = pm->monitor.pfnAddPort(pName, hWnd, pMonitorName);
         TRACE("got %d with %u (%s)\n", res, GetLastError(), debugstr_w(pm->dllname));
     }
     else
@@ -1664,8 +1702,11 @@ static BOOL WINAPI fpAddPortEx(LPWSTR pName, DWORD level, LPBYTE pBuffer, LPWSTR

/* load the Monitor */
     pm = monitor_load(pMonitorName, NULL);
-    if (pm && pm->monitor && pm->monitor->pfnAddPortEx) {
-        res = pm->monitor->pfnAddPortEx(pName, level, pBuffer, pMonitorName);
+    if (!pm) return FALSE;
+
+    if (pm->monitor.pfnAddPortEx)
+    {
+        res = pm->monitor.pfnAddPortEx(pName, level, pBuffer, pMonitorName);
         TRACE("got %d with %u (%s)\n", res, GetLastError(), debugstr_w(pm->dllname));
     }
     else
@@ -1778,10 +1819,13 @@ static BOOL WINAPI fpConfigurePort(LPWSTR pName, HWND hWnd, LPWSTR pPortName)
     }

pm = monitor_load_by_port(pPortName);
-    if (pm && pm->monitor && pm->monitor->pfnConfigurePort) {
+    if (!pm) return FALSE;
+
+    if (pm->monitor.pfnConfigurePort)
+    {
         TRACE("use %s for %s (monitor %p: %s)\n", debugstr_w(pm->name),
                 debugstr_w(pPortName), pm, debugstr_w(pm->dllname));
-        res = pm->monitor->pfnConfigurePort(pName, hWnd, pPortName);
+        res = pm->monitor.pfnConfigurePort(pName, hWnd, pPortName);
         TRACE("got %d with %u\n", res, GetLastError());
     }
     else
@@ -1908,10 +1952,13 @@ static BOOL WINAPI fpDeletePort(LPWSTR pName, HWND hWnd, LPWSTR pPortName)
     }

pm = monitor_load_by_port(pPortName);
-    if (pm && pm->monitor && pm->monitor->pfnDeletePort) {
+    if (!pm) return FALSE;
+
+    if (pm->monitor.pfnDeletePort)
+    {
         TRACE("use %s for %s (monitor %p: %s)\n", debugstr_w(pm->name),
                 debugstr_w(pPortName), pm, debugstr_w(pm->dllname));
-        res = pm->monitor->pfnDeletePort(pName, hWnd, pPortName);
+        res = pm->monitor.pfnDeletePort(pName, hWnd, pPortName);
         TRACE("got %d with %u\n", res, GetLastError());
     }
     else
@@ -2338,7 +2385,8 @@ static BOOL WINAPI fpXcvData(HANDLE hXcv, LPCWSTR pszDataName, PBYTE pInputData,

*pcbOutputNeeded = 0;

-    *pdwStatus = printer->pm->monitor->pfnXcvDataPort(printer->hXcv, pszDataName,
+    if (printer->pm->monitor.pfnXcvDataPort)
+        *pdwStatus = printer->pm->monitor.pfnXcvDataPort(printer->hXcv, pszDataName,
             pInputData, cbInputData, pOutputData, cbOutputData, pcbOutputNeeded);

return TRUE;

[Huw Davies]

Hi Dmitry,

On Thu, Sep 05, 2019 at 06:31:41PM +0800, Dmitry Timoshkov wrote:
> diff --git a/dlls/localspl/provider.c b/dlls/localspl/provider.c
> index 8e5e3428d5..44cf38e571 100644
> --- a/dlls/localspl/provider.c
> +++ b/dlls/localspl/provider.c
> @@ -577,15 +597,26 @@ static monitor_t * monitor_loadui(monitor_t * pm)
>      }
>  /* query the userinterface-dllname from the Portmonitor */
> - if ((pm->monitor) && (pm->monitor->pfnXcvDataPort)) {
> - if (pm->monitor.pfnXcvDataPort) {
> -
>    res = pm->monitor.pfnXcvOpenPort(0, emptyW, SERVER_ACCESS_ADMINISTER, &hXcv);
> -
>    TRACE("got %u with %p\n", res, hXcv);
> -
>    if (res) {
> -
>        res = pm->monitor.pfnXcvDataPort(hXcv, monitorUIW, NULL, 0, (BYTE *) buffer, sizeof(buffer), &len);
> -
>        TRACE("got %u with %s\n", res, debugstr_w(buffer));
> -
>        if (res == ERROR_SUCCESS) pui = monitor_load(NULL, buffer);
> -
>        pm->monitor.pfnXcvClosePort(hXcv);
> -
>    }
> -
>    return pui;
> - }
> - else if (pm->old_XcvDataPort) {
>     /* building (",XcvMonitor %s",pm->name) not needed yet */
> -
>    res = pm->monitor->pfnXcvOpenPort(emptyW, SERVER_ACCESS_ADMINISTER, &hXcv);
> -
>    res = pm->old_XcvOpenPort(emptyW, SERVER_ACCESS_ADMINISTER, &hXcv);
>    TRACE("got %u with %p\n", res, hXcv);
>    if (res) {
> -
>        res = pm->monitor->pfnXcvDataPort(hXcv, monitorUIW, NULL, 0, (BYTE *) buffer, sizeof(buffer), &len);
> -
>        res = pm->monitor.pfnXcvDataPort(hXcv, monitorUIW, NULL, 0, (BYTE *) buffer, sizeof(buffer), &len);
>        TRACE("got %u with %s\n", res, debugstr_w(buffer));
>        if (res == ERROR_SUCCESS) pui = monitor_load(NULL, buffer);
> -
>        pm->monitor->pfnXcvClosePort(hXcv);
> -
>        pm->monitor.pfnXcvClosePort(hXcv);
>    }
> }
>    return pui;

Thanks for doing this, but this bit doesn't look quite right.

You don't need old_XcvDataPort at all since the prototypes of old and new
are the same.  All you need to do is to call the appropriate XcvOpenPort
function and then fall into common code that calls XcvDataPort.  [And if
you really want to do it like you have, you need to call old_XvcDataPort
in the else block].

Huw.

[Dmitry Timoshkov]

Huw Davies  wrote:
> Thanks for doing this, but this bit doesn't look quite right.
>
> You don't need old_XcvDataPort at all since the prototypes of old and new
> are the same.  All you need to do is to call the appropriate XcvOpenPort
> function and then fall into common code that calls XcvDataPort.  [And if
> you really want to do it like you have, you need to call old_XvcDataPort
> in the else block].

Ah, right. Thanks for catching that.