Re: wtsapi32: partial implementation of WTSEnumerateProcessesW (resend)

[André Hentschel]

> diff --git a/dlls/wtsapi32/wtsapi32.c b/dlls/wtsapi32/wtsapi32.c
> index 898b95d..53c6325 100644
> --- a/dlls/wtsapi32/wtsapi32.c
> +++ b/dlls/wtsapi32/wtsapi32.c
> @@ -18,9 +18,13 @@
>  #include "config.h"
>  #include <stdarg.h>
>  #include <stdlib.h>
> -#include "windef.h"
> -#include "winbase.h"
> +#include "ntstatus.h"
> +#define WIN32_NO_STATUS
> +#include "winternl.h"
>  #include "wtsapi32.h"
> +#include "wine/library.h"
> +#include "wine/server.h"
> +#include "wine/unicode.h"
>  #include "wine/debug.h"

do you need that all?
> WINE_DEFAULT_DEBUG_CHANNEL(wtsapi);
> @@ -84,14 +88,65 @@ BOOL WINAPI WTSEnumerateProcessesA(HANDLE hServer, DWORD Reserved, DWORD Version
>  BOOL WINAPI WTSEnumerateProcessesW(HANDLE hServer, DWORD Reserved, DWORD Version,
>      PWTS_PROCESS_INFOW* ppProcessInfo, DWORD* pCount)
>  {
> - FIXME("Stub %p 0x%08x 0x%08x %p %p\n", hServer, Reserved, Version,
> -
>      ppProcessInfo, pCount);
> - SYSTEM_PROCESS_INFORMATION *spi;
> - ULONG size = 0x4000;
> - void *buf = NULL;
> - NTSTATUS status;
> - DWORD i,lcount = 0;
> - WTS_PROCESS_INFOW *linfo;

can't you use sizeof(*spi) or a multiple of it for size? or clean it up somehow else...
>  if (!ppProcessInfo || !pCount) return FALSE;
>
>  *pCount = 0;
>  *ppProcessInfo = NULL;
>
> - if (hServer != WTS_CURRENT_SERVER_HANDLE)
> - {
> -
>    FIXME("Only WTS_CURRENT_SERVER_HANDLE is impemented\n");
> -
>    return FALSE;
> - }
> -
> - do {
> -
>    size *= 2;
> -
>    HeapFree(GetProcessHeap(), 0, buf);
> -
>    buf = HeapAlloc(GetProcessHeap(), 0, size);
> -
>    if (!buf)
> -
>        return FALSE;
> -
> -
>    status = NtQuerySystemInformation(SystemProcessInformation, buf, size, NULL);
> - } while(status == STATUS_INFO_LENGTH_MISMATCH);
> -
> - if (status != STATUS_SUCCESS)
> - {
> -
>    HeapFree(GetProcessHeap(), 0, buf);
> -
>    SetLastError(RtlNtStatusToDosError(status));
> -
>    return FALSE;
> - }
> -
> - spi = buf;

why not dropping buf completely?
> - do {
> -
>    lcount++;
> -
>    spi = (SYSTEM_PROCESS_INFORMATION *)(((PCHAR)spi) + spi->NextEntryOffset);
> - } while(spi->NextEntryOffset != 0);

isn't there an easier way to get the count?
> - linfo = HeapAlloc(GetProcessHeap(), 0, lcount * sizeof(WTS_PROCESS_INFOW));
> - if(!linfo)
> -
>    return FALSE;
> -
> - for(i = 0, spi = buf; i < lcount; i++)
> - {
> -
>    linfo[i].SessionId = 0;   
> -
>    linfo[i].ProcessId = HandleToUlong(spi->UniqueProcessId);
> -
>    linfo[i].pProcessName = spi->ProcessName.Buffer;
> -
>    linfo[i].pUserSid = NULL;   
> -
>    spi = (SYSTEM_PROCESS_INFORMATION *)(((PCHAR)spi) + spi->NextEntryOffset);
> - }
> - *ppProcessInfo = linfo;
> - *pCount = lcount;
> - FIXME("pUserSid not filled\n");
> -
> - HeapFree(GetProcessHeap(), 0, buf);
> return TRUE;
>
> }
>
> diff --git a/include/wtsapi32.h b/include/wtsapi32.h
> index f4bab56..0476478 100644
> --- a/include/wtsapi32.h
> +++ b/include/wtsapi32.h
> @@ -135,6 +135,8 @@ typedef struct _WTS_SERVER_INFOW
>  DECL_WINELIB_TYPE_AW(WTS_SERVER_INFO)
>  DECL_WINELIB_TYPE_AW(PWTS_SERVER_INFO)
>
> +#define WTS_CURRENT_SERVER_HANDLE NULL
> -

maybe some more WTS_ defines while you're at it
> void WINAPI WTSCloseServer(HANDLE);
>  BOOL WINAPI WTSConnectSessionA(ULONG, ULONG, PSTR, BOOL);
>  BOOL WINAPI WTSConnectSessionW(ULONG, ULONG, PWSTR, BOOL);

wtsapi32: partial implementation of WTSEnumerateProcessesW (resend)

[Stefan Leichter]

Thursday 19 December 2013 André Hentschel
> > diff --git a/dlls/wtsapi32/wtsapi32.c b/dlls/wtsapi32/wtsapi32.c
> > index 898b95d..53c6325 100644
> > --- a/dlls/wtsapi32/wtsapi32.c
> > +++ b/dlls/wtsapi32/wtsapi32.c
> > @@ -18,9 +18,13 @@
> >
> > #include "config.h"
> >  #include <stdarg.h>
> >  #include <stdlib.h>
> >
> > -#include "windef.h"
> > -#include "winbase.h"
> > +#include "ntstatus.h"
> > +#define WIN32_NO_STATUS
> > +#include "winternl.h"
> >
> > #include "wtsapi32.h"
> >
> > +#include "wine/library.h"
> > +#include "wine/server.h"
> > +#include "wine/unicode.h"
> >
> > #include "wine/debug.h"
>
> do you need that all?

You are right, this can be done better
> > WINE_DEFAULT_DEBUG_CHANNEL(wtsapi);
> >
> > @@ -84,14 +88,65 @@ BOOL WINAPI WTSEnumerateProcessesA(HANDLE hServer,
> > DWORD Reserved, DWORD Version
> >
> > BOOL WINAPI WTSEnumerateProcessesW(HANDLE hServer, DWORD Reserved, DWORD
> >  Version,
> >  PWTS_PROCESS_INFOW* ppProcessInfo, DWORD* pCount)
> >
> > {
> > - FIXME("Stub %p 0x%08x 0x%08x %p %p\n", hServer, Reserved, Version,
> > -
> >      ppProcessInfo, pCount);
> > - SYSTEM_PROCESS_INFORMATION *spi;
> > - ULONG size = 0x4000;
> > - void *buf = NULL;
> > - NTSTATUS status;
> > - DWORD i,lcount = 0;
> > - WTS_PROCESS_INFOW *linfo;
>
> can't you use sizeof(*spi) or a multiple of it for size? or clean it up
> somehow else...

Of course sizeof(*spi) can be used.
> >  if (!ppProcessInfo || !pCount) return FALSE;
> >
> >  *pCount = 0;
> >  *ppProcessInfo = NULL;
> >
> > - if (hServer != WTS_CURRENT_SERVER_HANDLE)
> > - {
> > -
> >    FIXME("Only WTS_CURRENT_SERVER_HANDLE is impemented\n");
> > -
> >    return FALSE;
> > - }
> > -
> > - do {
> > -
> >    size *= 2;
> > -
> >    HeapFree(GetProcessHeap(), 0, buf);
> > -
> >    buf = HeapAlloc(GetProcessHeap(), 0, size);
> > -
> >    if (!buf)
> > -
> >        return FALSE;
> > -
> > -
> >    status = NtQuerySystemInformation(SystemProcessInformation, buf,
> >
> > size, NULL); +    } while(status == STATUS_INFO_LENGTH_MISMATCH);
> > -
> > - if (status != STATUS_SUCCESS)
> > - {
> > -
> >    HeapFree(GetProcessHeap(), 0, buf);
> > -
> >    SetLastError(RtlNtStatusToDosError(status));
> > -
> >    return FALSE;
> > - }
> > -
> > - spi = buf;
>
> why not dropping buf completely?

The main problem is that the value of NextEntryOffset is different from one spi 
to the next. Something like spi[i] can't be done. Therefore i need to keep the 
reference to the first SYSTEM_PROCESS_INFORMATION element.
> > - do {
> > -
> >    lcount++;
> > -
> >    spi = (SYSTEM_PROCESS_INFORMATION *)(((PCHAR)spi) +
> >
> > spi->NextEntryOffset); +    } while(spi->NextEntryOffset != 0);
>
> isn't there an easier way to get the count?

I don't see any
> > - linfo = HeapAlloc(GetProcessHeap(), 0, lcount *
> >
> > sizeof(WTS_PROCESS_INFOW)); +    if(!linfo)
> > -
> >    return FALSE;
> > -
> > - for(i = 0, spi = buf; i < lcount; i++)
> > - {
> > -
> >    linfo[i].SessionId = 0;
> > -
> >    linfo[i].ProcessId = HandleToUlong(spi->UniqueProcessId);
> > -
> >    linfo[i].pProcessName = spi->ProcessName.Buffer;
> > -
> >    linfo[i].pUserSid = NULL;
> > -
> >    spi = (SYSTEM_PROCESS_INFORMATION *)(((PCHAR)spi) +
> >
> > spi->NextEntryOffset); +    }
> > -
> >
> > *ppProcessInfo = linfo;
> > -
> >
> > *pCount = lcount;
> > -
> >
> > FIXME("pUserSid not filled\n");
> > -
> > -
> >
> > HeapFree(GetProcessHeap(), 0, buf);
> >
> > return TRUE;
> >
> > }
> >
> > diff --git a/include/wtsapi32.h b/include/wtsapi32.h
> > index f4bab56..0476478 100644
> > --- a/include/wtsapi32.h
> > +++ b/include/wtsapi32.h
> > @@ -135,6 +135,8 @@ typedef struct _WTS_SERVER_INFOW
> >
> > DECL_WINELIB_TYPE_AW(WTS_SERVER_INFO)
> >  DECL_WINELIB_TYPE_AW(PWTS_SERVER_INFO)
> >
> > +#define WTS_CURRENT_SERVER_HANDLE NULL
> > -
>
> maybe some more WTS_ defines while you're at it

I though we add the content of the header files on demand. The prototyp of 
WTSEnumerateProcessesW is already in the header file. What do you have in mind?
> > void WINAPI WTSCloseServer(HANDLE);
> >  BOOL WINAPI WTSConnectSessionA(ULONG, ULONG, PSTR, BOOL);
> >  BOOL WINAPI WTSConnectSessionW(ULONG, ULONG, PWSTR, BOOL);