Re: [PATCH 2/2] winspool+localspl: Move AddPrinterW implementation to localspl

[Huw Davies]

On Thu, Jan 26, 2017 at 06:24:38PM +0100, Detlef Riekenberg wrote:
> We need to move AddPrinter to allow importing CUPS printers
> centralized in the spooler service. (bug 3507, bug 10358)
>
> localspl will be used from the application (through winspool.drv)
> and from the spooler service (through spoolss.dll)
>
> The changes before and after the code move are as minimal as possible

Hi Detlef,

Could you explain a bit more about where this going?  How much more will
need to move to localspl to enable this to happen?  Is it possible for
the spooler service to use winspool.drv instead of spoolss.dll?

I've also inlined a few comments below.
> diff --git a/dlls/localspl/provider.c b/dlls/localspl/provider.c
> index f359b73..9b12c63 100644
> --- a/dlls/localspl/provider.c
> +++ b/dlls/localspl/provider.c
> @@ -93,13 +93,22 @@ static monitor_t * pm_localport;
> -static const WCHAR spoolW[] = {'\\','s','p','o','o','l',0};
> -static const WCHAR driversW[] = {'\\','d','r','i','v','e','r','s','\\',0};
> +static const WCHAR printer_driverW[] = {'P','r','i','n','t','e','r',' ','D','r','i','v','e','r',0};
> +static const WCHAR priorityW[] = {'P','r','i','o','r','i','t','y',0};
> +static const WCHAR providerW[] = {'P','r','o','v','i','d','e','r',0};

providerW is unused.
> +static HKEY localspl_OpenDriverReg(LPCWSTR pEnvironment)
> +{
> +    HKEY retval = NULL;
> +    LPWSTR buffer;
> +    const printenv_t * env;
> +
> +    TRACE("(%s)\n", debugstr_w(pEnvironment));
> +
> +    env = validate_envW(pEnvironment);
> +    if (!env) return NULL;
> +
> +    buffer = heap_alloc((strlenW(fmt_driversW) + strlenW(env->envname) + strlenW(env->versionregpath) + 1) * sizeof(WCHAR));
> +
> +    if(buffer) {

I know this is copied from winspool.drv, but let's try to be consistent
with the formatting and put that brace on a new line.  Same for other K&R
style hunks below.
> +static WINAPI DEVMODEW * get_default_devmode(LPWSTR printername, LPWSTR drivername)
> +{
> +
> +    LONG size;
> +    HMODULE hwinspool;
> +    DEVMODEW * dm = NULL;
> +    LONG (WINAPI * pDocumentPropertiesW)(HWND hWnd, HANDLE hPrinter, LPWSTR pDeviceName,
> +                                         LPDEVMODEW pDevModeOutput, LPDEVMODEW pDevModeInput, DWORD fMode);
> +
> +    TRACE("printername: %s, drivername: %s\n", debugstr_w(printername), debugstr_w(drivername));
> +
> +    hwinspool = LoadLibraryW(winspooldrvW);
> +    if (!hwinspool) return NULL;
> +
> +    pDocumentPropertiesW = (void *) GetProcAddress(hwinspool, "DocumentPropertiesW");

Calling back to winspool.drv seems awkward.  You could simply have winspool.drv set
pi->pDevMode before calling localspl.  Of course, that leaves the question of what
spoolss should do, but that depends on what your answers are to the first questions.

Huw.