Re: Implementation of EnumMoniker and Partial impl on MkParseDisplayName

[Robert Shearman]

Jeff Latimer wrote:
> This is the first installment of the implementation of 
> MkParseDisplayName.  I would like feed back on style and usage.

Ok, I'll happily add comments on the patch.
> I think the implementation of the EnumMoniker interface looks ok.  
> Comments appreciated.
>
> Jeff Latimer
>
> ------------------------------------------------------------------------
>
> Index: moniker.c
> ===================================================================
> RCS file: /home/wine/wine/dlls/ole32/moniker.c,v
> retrieving revision 1.38
> diff -u -r1.38 moniker.c
> --- moniker.c	13 Dec 2004 21:19:02 -0000	1.38
> +++ moniker.c	28 Mar 2005 12:06:51 -0000
> @@ -29,7 +29,6 @@
>
> #include <assert.h>
> #include <stdarg.h>
> -#include <string.h>
>
> #define COBJMACROS
>
> @@ -38,6 +37,7 @@
> #include "winbase.h"
> #include "winuser.h"
> #include "wtypes.h"
> +#include "wine/unicode.h"
> #include "wine/debug.h"
> #include "ole2.h"
>
> @@ -50,11 +50,11 @@
> /* define the structure of the running object table elements */
> typedef struct RunObject{
>
> -    IUnknown*  pObj; /* points on a running object*/
> -    IMoniker*  pmkObj; /* points on a moniker who identifies this object */
> +    IUnknown*  pObj; 		/* points on a running object*/
> +    IMoniker*  pmkObj; 		/* points on a moniker who identifies this object */
>     FILETIME   lastModifObj;
> -    DWORD      identRegObj; /* registration key relative to this object */
> -    DWORD      regTypeObj; /* registration type : strong or weak */
> +    DWORD      identRegObj; 	/* registration key relative to this object */
> +    DWORD      regTypeObj; 	/* registration type : strong or weak */
> }RunObject;
>
> /* define the RunningObjectTableImpl structure */
> @@ -106,6 +106,55 @@
>     RunningObjectTableImpl_EnumRunning
> };
>
> +/* define the EnumMonikerImpl structure */
> +typedef struct EnumMonikerImpl{
> +
> +    IEnumMonikerVtbl *lpVtbl;
> +    ULONG      ref;
> +
> +    RunObject* TabMoniker;    /* pointer to the first object in the table       */
> +    DWORD      TabSize;       /* current table size                             */
> +    DWORD      TabLastIndx;   /* first free index element in the table.         */

Since you asked for feedback on style, I'll say that you should try to 
keep the naming conventions consistent.
> +    DWORD      runObjTabRegister; /* registration key of the next registered object */
> +    DWORD      currentPos;    /* enum position in the list			*/
> +
> +} EnumMonikerImpl;
> +
> +
> +/* IEnumMoniker prototype functions : */
> +/* IUnknown functions*/
> +static HRESULT WINAPI EnumMonikerImpl_QueryInterface(IEnumMoniker* iface,REFIID riid,void** ppvObject);
> +static ULONG   WINAPI EnumMonikerImpl_AddRef(IEnumMoniker* iface);
> +static ULONG   WINAPI EnumMonikerImpl_Release(IEnumMoniker* iface);
> +/* IEnumMoniker functions */
> +static HRESULT WINAPI EnumMonikerImpl_Next(IEnumMoniker* iface, ULONG celt, IMoniker** rgelt, ULONG* pceltFetched);
> +static HRESULT WINAPI EnumMonikerImpl_Skip(IEnumMoniker* iface, ULONG celt);
> +static HRESULT WINAPI EnumMonikerImpl_Reset(IEnumMoniker* iface);
> +static HRESULT WINAPI EnumMonikerImpl_Clone(IEnumMoniker* iface, IEnumMoniker** ppenum);

If you put the vtable after the implementations of the functions then 
you don't need to declare the prototypes here.
> +
> +/* Local functions*/
> +HRESULT WINAPI EnumMonikerImpl_Initialize(void);
> +HRESULT WINAPI EnumMonikerImpl_CreateEnumROTMoniker(RunObject* runObjTab,
> +                                                 ULONG TabSize,
> +						 ULONG TabLastIndx,
> +                                                 ULONG currentPos,
> +                                                 IEnumMoniker ** ppenumMoniker);
> +HRESULT WINAPI EnumMonikerImpl_UnInitialize(void);
> +HRESULT WINAPI EnumMonikerImpl_Destroy(void);
> +HRESULT WINAPI EnumMonikerImpl_GetObjectIndex(EnumMonikerImpl* This,DWORD identReg,IMoniker* pmk,DWORD *indx);

Presumably, these should all be static since they are not used outside 
the file.
> +
> +/* Virtual function table for the IEnumMoniker class. */
> +static IEnumMonikerVtbl VT_EnumMonikerImpl =
> +{
> +    EnumMonikerImpl_QueryInterface,
> +    EnumMonikerImpl_AddRef,
> +    EnumMonikerImpl_Release,
> +    EnumMonikerImpl_Next,
> +    EnumMonikerImpl_Skip,
> +    EnumMonikerImpl_Reset,
> +    EnumMonikerImpl_Clone
> +};
> +
> /***********************************************************************
>  *        RunningObjectTable_QueryInterface
>  */
> +/***********************************************************************
> + *        EnumMonikerImpl_CreateEnumROTMoniker
> + *        Used by EnumRunning to create the structure and EnumClone
> + *	  to copy the structure
> + */
> +HRESULT WINAPI EnumMonikerImpl_CreateEnumROTMoniker(RunObject* TabMoniker,
> +                                                 ULONG TabSize,
> +						 ULONG TabLastIndx,
> +                                                 ULONG currentPos,
> +                                                 IEnumMoniker ** ppenumMoniker)
> +{
> +    int i;
> +    if (currentPos > TabSize)
> +	return E_INVALIDARG;
> +
> +    if (ppenumMoniker == 0)
> +        return E_OUTOFMEMORY;

This should probably be E_INVALIDARG.
> +
> +    *ppenumMoniker = HeapAlloc(GetProcessHeap(), 0, sizeof(EnumMonikerImpl));
> +

if (!*ppenumMoniker) return E_OUTOFMEMORY;
> +    void *vpThis=*ppenumMoniker;

Variable used just for changing types?!?!
> +    EnumMonikerImpl* This=0;

Using NULL instead of 0 tends to make it clearer that you are using a 
pointer.
> +    This=vpThis;

All of these variable declarations need moving to the top of the block.
> +    TRACE("(%p)\n",This);
> +    /* initialize the virtual table function */
> +    This->lpVtbl = &VT_EnumMonikerImpl;
> +
> +    /* the initial reference is set to "1" because if set to "0" it will be not practis when */
> +    /* the ROT referred many times not in the same time (all the objects in the ROT will */
> +    /* be removed every time the ROT is removed ) */
> +    This->ref = 1;			/* set the ref count to one         */
> +    This->currentPos=0;			/* Set the list start posn to start */
> +    This->TabSize=TabSize; 		/* Need the same size table as ROT */
> +    This->TabLastIndx=TabLastIndx; 	/* end element */
> +    This->TabMoniker=HeapAlloc(GetProcessHeap(),0,This->TabSize*sizeof(RunObject));
> +
> +    if (This->TabMoniker==NULL) {
> +        HeapFree(GetProcessHeap(), 0, This);
> +        return E_OUTOFMEMORY;
> +    }
> +    for (i=0;i<This->TabLastIndx;i++){
> +
> +        This->TabMoniker[i]=TabMoniker[i];
> +        IMoniker_AddRef(TabMoniker[i].pmkObj);
> +    }
> +
> +    return S_OK;
> }
>
> /***********************************************************************
> @@ -512,8 +621,180 @@
>     return res;
> }
>
> +/***********************************************************************
> + *        EnumMoniker_QueryInterface
> + */
> +HRESULT WINAPI EnumMonikerImpl_QueryInterface(IEnumMoniker* iface,REFIID riid,void** ppvObject)
> +{
> +    EnumMonikerImpl *This = (EnumMonikerImpl *)iface;
> +
> +    TRACE("(%p,%p,%p)\n",This,riid,ppvObject);
> +
> +    /* validate arguments */
> +    if (This==0)
> +        return CO_E_NOTINITIALIZED;

Remove this check. It is completely bogus.
> +
> +    if (ppvObject==0)
> +        return E_INVALIDARG;
> +
> +    *ppvObject = 0;
> +
> +    if (IsEqualIID(&IID_IUnknown, riid))
> +        *ppvObject = (IEnumMoniker*)This;
> +    else
> +        if (IsEqualIID(&IID_IEnumMoniker, riid))
> +            *ppvObject = (IEnumMoniker*)This;
> +
> +    if ((*ppvObject)==0)
> +        return E_NOINTERFACE;
> +
> +    EnumMonikerImpl_AddRef(iface);
> +
> +    return S_OK;
> +}
> +
> +/***********************************************************************
> + *        EnumMoniker_AddRef
> + */
> +ULONG   WINAPI EnumMonikerImpl_AddRef(IEnumMoniker* iface)
> +{
> +    EnumMonikerImpl *This = (EnumMonikerImpl *)iface;
> +
> +    TRACE("(%p)\n",This);
> +
> +    return InterlockedIncrement(&This->ref);
> +}
> +
> +/***********************************************************************
> + *        EnumMoniker_Destroy
> + */
> +HRESULT WINAPI EnumMonikerImpl_Destroy()
> +{
> +    TRACE("(?)\n");
> +
> +//    if (enumMonikerInstance==NULL)
> +//        return E_INVALIDARG;
> +
> +    /* free the enummoniker structure memory */
> +//    HeapFree(GetProcessHeap(),0,enumMonikerInstance);
> +
> +    return S_OK;
> +}

You are not using this function, so remove it.
> +
> +/***********************************************************************
> + *        EnumMoniker_release
> + */
> +ULONG   WINAPI EnumMonikerImpl_Release(IEnumMoniker* iface)
> +{
> +    DWORD i;
> +    EnumMonikerImpl *This = (EnumMonikerImpl *)iface;
> +    ULONG ref;
> +
> +    TRACE("(%p)\n",This);
> +
> +    if (This==0)
> +        return E_INVALIDARG;

Another bogus This pointer check.
> +
> +    ref = InterlockedDecrement(&This->ref);
> +
> +    /* unitialize rot structure if there's no more reference to it*/
> +    if (ref == 0) {
> +
> +        /* release all registered objects in Moniker list */
> +        for(i=0;i<This->TabLastIndx;i++)
> +        {
> +            IMoniker_Release(This->TabMoniker[i].pmkObj);
> +        }
> +
> +        /* there're no more elements in the table */
> +
> +	TRACE("(%p) Deleting\n",This);
> +	HeapFree (GetProcessHeap(), 0, This->TabMoniker); // free Moniker list
> +	HeapFree (GetProcessHeap(), 0, This);		  // free Enum Instance
> +	
> +    }
> +
> +    return ref;
> +}
> +/***********************************************************************
> + *        EnmumMoniker_Next
> + */
> +HRESULT   WINAPI EnumMonikerImpl_Next(IEnumMoniker* iface, ULONG celt, IMoniker** rgelt, ULONG * pceltFetched)
> +{
> +//    DWORD i;
> +//    ULONG ref;

Remove these lines.
> +
> +    EnumMonikerImpl *This = (EnumMonikerImpl *)iface;
> +    TRACE("(%p) currentPos %d Tablastindx %d\n",This, (int)This->currentPos, (int)This->TabLastIndx);
> +    ULONG i;
> +
> +    /* retrieve the requested number of moniker from the current position */
> +    for(i=0;((This->currentPos < This->TabLastIndx) && (i < celt));i++)
> +	rgelt[i]=(IMoniker*)This->TabMoniker[This->currentPos++].pmkObj;
> +
> +    if (pceltFetched!=NULL)
> +        *pceltFetched= i;
> +
> +    if (i==celt)
> +        return S_OK;
> +    else
> +        return S_FALSE;
> +
> +}
> +
> +/***********************************************************************
> + *        EnmumMoniker_Skip
> + */
> +HRESULT   WINAPI EnumMonikerImpl_Skip(IEnumMoniker* iface, ULONG celt)
> +{
> +    EnumMonikerImpl *This = (EnumMonikerImpl *)iface;
> +
> +    TRACE("(%p)\n",This);
> +    if (This==0)
> +        return E_INVALIDARG;

Another bogus check. I'll stop commenting about these now, but you 
should remove all of them.
> +
> +    if  ((This->currentPos+celt) >= This->TabLastIndx)
> +	return S_FALSE;
> +
> +    This->currentPos+=celt;
> +
> +    return S_OK;
> +}
> +
> +/***********************************************************************
> + *        EnmumMoniker_Reset
> + */
> +HRESULT   WINAPI EnumMonikerImpl_Reset(IEnumMoniker* iface)
> +{
> +    EnumMonikerImpl *This = (EnumMonikerImpl *)iface;
> +    if (This==0)
> +        return E_INVALIDARG;
> +
> +    This->currentPos = 0;	/* set back to start of list */
> +
> +    TRACE("(%p)\n",This);
> +
> +    return S_OK;
> +}
> +
> +/***********************************************************************
> + *        EnmumMoniker_Clone
> + */
> +HRESULT   WINAPI EnumMonikerImpl_Clone(IEnumMoniker* iface, IEnumMoniker ** ppenum)
> +{
> +    EnumMonikerImpl *This = (EnumMonikerImpl *)iface;
> +    if (This==0)
> +        return E_INVALIDARG;
> +
> +    TRACE("(%p)\n",This);
> +    /* copy the enum structure */ 
> +    return EnumMonikerImpl_CreateEnumROTMoniker(This->TabMoniker, This->TabSize,
> +					 This->TabLastIndx, This->currentPos,
> +					 ppenum);
> +}
> +
> /******************************************************************************
> - *              OleRun        [OLE32.@]
> + *              olerun        [ole32.@]

The name of the function really is OleRun, not olerun. Feel free to 
submit the change from OLE32->ole32 in another patch, but make sure you 
change every instance of this.
> */
> HRESULT WINAPI OleRun(LPUNKNOWN pUnknown)
> {
> @@ -523,22 +804,142 @@
>
> ret = IRunnableObject_QueryInterface(This,&IID_IRunnableObject,(LPVOID*)&runable);
>   if (ret)
> -	return 0; /* Appears to return no error. */
> +	return 0; /* appears to return no error. */
>   ret  = IRunnableObject_Run(runable,NULL);
>   IRunnableObject_Release(runable);
>   return ret;
> }
>
> /******************************************************************************
> - *              MkParseDisplayName        [OLE32.@]
> + *              MkParseDisplayName        [ole32.@]
>  */
> HRESULT WINAPI MkParseDisplayName(LPBC pbc, LPCOLESTR szUserName,
>       		LPDWORD pchEaten, LPMONIKER *ppmk)
> {
> -    FIXME("(%p, %s, %p, %p): stub.\n", pbc, debugstr_w(szUserName), pchEaten, *ppmk);
> +    int char_cnt = 0;
> +    int usernamelen = 0;
> +    HRESULT rc =0;
> +    WCHAR  username [2048];			// workspace to parse name into

Please don't use C99 comments. Use "/* comment */" instead.
> +    IRunningObjectTable * pprot;
> +    IEnumMoniker *spEM;
> +    IMoniker * spMoniker;
> +//    IUnknown  * spUnk;
> +    LPMALLOC *  ppMalloc;
> +    TRACE("(%p)\n", pbc);
> +    rc = CoGetMalloc(1, (LPMALLOC *) &ppMalloc);

You leak references to ppMalloc everywhere. Is it really necessary to 
use CoGetMalloc?
> if (!(IsValidInterface((LPUNKNOWN) pbc)))
>   return E_INVALIDARG;
> +    usernamelen = (int) lstrlenW(szUserName); 	// overall string len
> +
>
> +    if (szUserName[char_cnt] == (WCHAR) '@') { 	// This is a progid not a file name
> +	FIXME("Progid not implemented\n");
> +	 return MK_E_SYNTAX;
> +    } else {
> +	char_cnt = 0;
> +    	if (szUserName[char_cnt+1] == ':') {
> +	    if ((szUserName[char_cnt] < 'a' || szUserName[char_cnt] > 'z') &&
> +		(szUserName[char_cnt] < 'A' || szUserName[char_cnt] > 'Z')) {
> +                FIXME("bad drive %c\n", szUserName[char_cnt]);

This should be an ERR instead of FIXME, unless there is something you 
haven't implemented. If that is the case, please add a comment here as 
to what that could be.
> +	    	return MK_E_SYNTAX;
> +            }
> +	    username[0] = (WCHAR)szUserName[0];
> +	    username[1] = szUserName[1];
> +	    char_cnt += 2;			// point past drive spec
> +	}
> +	for (;  szUserName[char_cnt] != 0x0 && 	// until end of string or
> +		szUserName[char_cnt] != '!' ; char_cnt++) { // end of component
> +	    FIXME("%c",szUserName[char_cnt]);

Is this just a debugging statement?
> +	    if (szUserName[char_cnt] == '\\' && szUserName[char_cnt+1] == '\\') {
> +	    	FIXME("badchar %c\n",szUserName[char_cnt]);
> +	    	return MK_E_SYNTAX;
> +	    }	
> +	    if (szUserName[char_cnt] == '/' ||
> +	    	szUserName[char_cnt] == '?' ||
> +	    	szUserName[char_cnt] == '<' ||
> +	    	szUserName[char_cnt] == '>' ||
> +	    	szUserName[char_cnt] == '*' ||
> +	    	szUserName[char_cnt] == '|' ||
> +	    	szUserName[char_cnt] == ':')
> +	   	{
> +	    	FIXME("badchar %c\n",szUserName[char_cnt]);
> +	    	return MK_E_SYNTAX;
> +	    }
> +	    username[char_cnt] = (WCHAR) szUserName[char_cnt];

Shouldn't need a cast here.
> +	}
> +	FIXME("\n");
> +	username[char_cnt] = 0x0;		// terminate the substring
> +	TRACE("File name to be converted to Moniker: %s\n", debugstr_w(username));
> +	rc = IBindCtx_GetRunningObjectTable(pbc, &pprot);
> +	if (!SUCCEEDED(rc))
> +	{
> +	    ERR("IBindCtx_GetRunningObjectTable failed %04x\n", (int) rc);

This should be:

+	    ERR("IBindCtx_GetRunningObjectTable failed 0x%08lx\n", rc);
> +	    return MK_E_SYNTAX;
> +	}
> +	rc = IRunningObjectTable_EnumRunning(pprot, &spEM);
> +	TRACE("IRunningObjectTable_EnumRunning rc:%04x %p\n", (int)rc, spEM);
>
> +	if  (SUCCEEDED(rc))
> +	{	
> +	    rc = MK_E_NOOBJECT;
> +	    while ((IEnumMoniker_Next(spEM, 1, &spMoniker, NULL)==S_OK) &&
> +		   (*ppmk ==NULL)) 
> +	    {
> +	        WCHAR *szDisplayn=NULL;
> +	        rc=IMoniker_GetDisplayName(spMoniker, pbc, NULL, 
> +					   (LPOLESTR*) &szDisplayn);
> +		TRACE("Display Name %s for Moniker %p\n", debugstr_w(szDisplayn), spMoniker);
> +	        if (SUCCEEDED(rc)) 
> +		{
> +	  	    TRACE("IMoniker_GetDisplayName succeeded, rc %08x ROT Name:%s, New name:%s\n", 
> +		         (int)rc, debugstr_w(szDisplayn), debugstr_w(username));
> +		    if (0==strcmpiW((WCHAR *)szDisplayn, (WCHAR *)username))
> +		    {
> +		    	TRACE("is Equal %p\n", spMoniker);
> +			*ppmk = spMoniker;	// we already have the Moniker, so use it
> +			IMoniker_AddRef(spMoniker); // say we are reusing this Moniker
> +		    }
> +		    else
> +		       TRACE("Not Equal\n");
> +
> +		    CoTaskMemFree(szDisplayn);
> +		}
> +            }
> +//	    rc = IRunningObjectTable_GetObject(pprot, spMoniker, &spUnk);
> +//	    if (!SUCCEEDED(rc)) 
> +//	    	TRACE("ok\n");
> +	    IEnumMoniker_Release(spEM);		// Free Enum interface
> +	    IRunningObjectTable_Release(pprot); // Decrement ROT count
> +	    pprot = NULL;			// Finished with ROT pointer		
> +	}
> +	else
> +	    ERR("IRunningObjectTable_EnumRunning failed %04x\n", (int ) rc);
> +
> +	if  (*ppmk == NULL)
> +	{
> +	    rc = CreateFileMoniker(username, ppmk); // convert to moniker
> +	    if (SUCCEEDED(rc))
> +	    {
> +		*pchEaten = (int) char_cnt;
> +		if (usernamelen > char_cnt) 	// there is more to do and not implemented
> +		{
> +		    TRACE("Username len %i exceeds parsed text %i\n", usernamelen, char_cnt);

This should be a FIXME then.
> +		    return MK_E_SYNTAX;
> +		} 
> +		TRACE(" CreateFileMoniker Succeeded %04x\n", (int) rc);	
> +		return (S_OK);
> +	    } else
> +	    {
> +		TRACE("CreateFileMoniker Failed %04x\n", (int) rc);

ERR
> +		return(rc);
> +	    }
> +	}
> +	else
> +	{
> +	    *pchEaten = (int) char_cnt;
> +	    TRACE(" Reused Moniker %p\n", *ppmk);	
> +	    return (S_OK); 
> +	}  
> +    }	
>     return MK_E_SYNTAX;
> }
>
> Index: Makefile.in
> ===================================================================
> RCS file: /home/wine/wine/dlls/ole32/Makefile.in,v
> retrieving revision 1.43
> diff -u -r1.43 Makefile.in
> --- Makefile.in	17 Mar 2005 20:50:35 -0000	1.43
> +++ Makefile.in	28 Mar 2005 12:06:52 -0000
> @@ -5,7 +5,7 @@
> VPATH     = @srcdir@
> MODULE    = ole32.dll
> IMPORTS   = advapi32 user32 gdi32 rpcrt4 kernel32 ntdll
> -EXTRALIBS = -luuid $(LIBUNICODE)
> +EXTRALIBS = -luuid $(LIBUNICODE)
>
> C_SRCS = \
>   antimoniker.c \

Useless whitespace only change.
> Index: tests/moniker.c
> ===================================================================
> RCS file: /home/wine/wine/dlls/ole32/tests/moniker.c,v
> retrieving revision 1.2
> diff -u -r1.2 moniker.c
> --- tests/moniker.c	28 Jan 2005 12:39:13 -0000	1.2
> +++ tests/moniker.c	28 Mar 2005 12:06:56 -0000
> @@ -22,40 +22,166 @@
> #define COBJMACROS
>
> #include <stdarg.h>
> +//#include <wchar.h>

Remove this line.
> #include "windef.h"
> #include "winbase.h"
> #include "objbase.h"
> +#include "objidl.h"
> #include "comcat.h"
>
> #include "wine/test.h"
>
> #define ok_ole_success(hr, func) ok(hr == S_OK, #func " failed with error 0x%08lx\n", hr)
>
> +LPTSTR * prt(HRESULT hr, LPTSTR * lpMsgBuf);

Not used anywhere.
> +
> static void test_MkParseDisplayName()
> {
>     IBindCtx * pbc = NULL;
>     HRESULT hr;
> -    IMoniker * pmk = NULL;
>     ULONG eaten;
> -    IUnknown * object = NULL;
> +    int cnt;
> +
>     /* CLSID of My Computer */
> -    static const WCHAR wszDisplayName[] = {'c','l','s','i','d',':',
> +    static const WCHAR wszDisplayName1[] = {'@','c','l','s','i','d',':',
> +        '2','0','D','0','4','F','E','0','-','3','A','E','A','-','1','0','6','9','-','A','2','D','8','-','0','8','0','0','2','B','3','0','3','0','9','D',':',0};

Does "clsid:..." without the "@" still work? I'd prefer to see that test 
still in there.
> +
> +    hr = CreateBindCtx(0, &pbc);
> +    ok_ole_success(hr, CreateBindCtx);
> +    IMoniker *pmk1 = NULL;
> +    hr = MkParseDisplayName(pbc, wszDisplayName1, &eaten, &pmk1);
> +    todo_wine { ok(hr==0x800401e4, "MkParseDisplayName - Progid not implemented hr=%08x\n", (int) hr); }

MK_E_SYNTAX. See the ok_ole_success macro for how to print out HRESULTs 
without casts.
> +
> +    IBindCtx_Release(pbc);
> +
> +    /* A bad drive */
> +    static const WCHAR wszDisplayName2[] = {'1',':','s','i','d',':',
>         '2','0','D','0','4','F','E','0','-','3','A','E','A','-','1','0','6','9','-','A','2','D','8','-','0','8','0','0','2','B','3','0','3','0','9','D',':',0};
>
> hr = CreateBindCtx(0, &pbc);
>     ok_ole_success(hr, CreateBindCtx);
> +    IMoniker *pmk2=NULL;
> +    hr = MkParseDisplayName(pbc, wszDisplayName2, &eaten, &pmk2);
> +    ok(hr==0x800401e4, "MkParseDisplayName hr=%08x\n", (int) hr); 
> +
> +    IBindCtx_Release(pbc);
>
> -    hr = MkParseDisplayName(pbc, wszDisplayName, &eaten, &pmk);
> -    todo_wine { ok_ole_success(hr, MkParseDisplayName); }
> +    /* A good drive, bad char */
> +    static const WCHAR wszDisplayName3[] = {'b',':','s','i','d',':',
> +        '2','0','D','0','4','F','E','0','-','3','A','E','A','-','1','0','6','9','-','A','2','D','8','-','0','8','0','0','2','B','3','0','3','0','9','D',':',0};
> +
> +    hr = CreateBindCtx(0, &pbc);
> +    ok_ole_success(hr, CreateBindCtx);
> +    IMoniker *pmk3=NULL;
> +    hr = MkParseDisplayName(pbc, wszDisplayName3, &eaten, &pmk3);
> +    todo_wine { ok(hr==0x800401e4, "MkParseDisplayName hr=%08x\n", (int) hr); }
> +
> +    IBindCtx_Release(pbc);
> +
> +    // Good file name
> +    static const WCHAR wszDisplayName4[] = {'c',':','\\','w','i','n','d',
> +        'o','w','s','\\','f','r','E','d','.','e','x','E',0x0};
> +
> +    hr = CreateBindCtx(0, &pbc);
> +    ok_ole_success(hr, CreateBindCtx);
> +
> +    IMoniker *pmk4=NULL;
> +    eaten=0;
> +    hr = MkParseDisplayName(pbc, wszDisplayName4, &eaten, &pmk4);
> +    todo_wine { ok(hr==0, "MkParseDisplayName hr=%08x\n", (int) hr); }
> +
> +    IBindCtx_Release(pbc);
> +
> +    /*  Good file name + item.  This test is to see if the whole display
> +     *  name is consumed
> +     */
> +    static const WCHAR wszDisplayName5[] = {'c',':','\\','w','i','n','d',
> +        'o','w','s','\\','f','r','E','d','.','e','x','E','!','m','a','r','k',0x0};
> +
> +    hr = CreateBindCtx(0, &pbc);
> +    ok_ole_success(hr, CreateBindCtx);
> +
> +    IMoniker *pmk5=NULL;
> +    eaten=0;
> +    hr = MkParseDisplayName(pbc, wszDisplayName5, &eaten, &pmk5);
> +    ok(hr!=0, "MkParseDisplayName failed hr=%08x\n", (int) hr); 
> +    cnt = (int) lstrlenW(wszDisplayName5); // Get length of text
> +    ok(eaten==cnt, "eaten %d string len %i\n", (int) eaten, cnt); // See if consiumed
> +
> +    IBindCtx_Release(pbc);
> +
> +    /*  This test is to create a valid Moniker an then load a couple of 
> +     *  entries into the ROT to test that MkParseDisplayName reuses the
> +     *  Moniker in the ROT.  Currently using the EnumMoniker object as 
> +     *  part of the Register call to satisfy the syntax
> +     */
> +
> +    static const WCHAR wszDisplayName6[] = {'c',':','\\','P','r','o','g',
> +        'r','a','m',' ','F','i','l','e','s','\\','v','i','m','\\',
> +	'v','i','m','6','1','\\','g','v','i','m','.','e','x','e',0x0};
> +    hr = CreateBindCtx(0, &pbc);
> +    ok_ole_success(hr, CreateBindCtx);
>
> -    if (object)
> -    {
> -        hr = IMoniker_BindToObject(pmk, pbc, NULL, &IID_IUnknown, (LPVOID*)&object);
> -        ok_ole_success(hr, IMoniker_BindToObject);
> +    IMoniker *pmk6=NULL;
> +    eaten=0;
> +    hr = MkParseDisplayName(pbc, wszDisplayName6, &eaten, &pmk6);
> +    ok(hr==0, "MkParseDisplayName hr=%08x\n", (int) hr);
> +    cnt = lstrlenW(wszDisplayName6); // Get length of text
> +    ok(eaten==cnt, "eaten %d string len %i\n", (int) eaten, cnt); // See if consiumed
> +
> +    IRunningObjectTable * pprot=NULL;
> +    hr = IBindCtx_GetRunningObjectTable(pbc, &pprot);
> +    todo_wine { ok(hr==0, "IBindCtx_GetRunningObjectTable hr=%08x\n", (int) hr); }
> +    IEnumMoniker *spEM1=NULL;
> +    hr = IRunningObjectTable_EnumRunning(pprot, &spEM1);
> +    todo_wine { ok(hr==0, "IRunningObjectTable_EnumRunning hr=%08x\n", (int) hr);}
> +    IUnknown *lpEM1;
> +    hr = IEnumMoniker_QueryInterface(spEM1, &IID_IUnknown, (void*) &lpEM1);
> +    todo_wine { ok(hr==0, "IEnumMoniker_QueryInterface hr %08x %p\n", (int) hr, lpEM1); }
> +    DWORD pdwReg1=0;
> +    DWORD grflags=0;
> +    grflags= grflags | ROTFLAGS_REGISTRATIONKEEPSALIVE;
> +    hr = IRunningObjectTable_Register(pprot, grflags, lpEM1, pmk5, &pdwReg1);    
> +    todo_wine { ok(hr==0, "IRunningObjectTable_Register hr=%08x %p %08x %p %p %d\n", 
> +		(int) hr, pprot, (int) grflags, lpEM1, pmk5, (int) pdwReg1); }
> +
> +    DWORD pdwReg2=0;
> +    grflags=0;
> +    grflags= grflags | ROTFLAGS_REGISTRATIONKEEPSALIVE;
> +    hr = IRunningObjectTable_Register(pprot, grflags, lpEM1, pmk6, &pdwReg2);    
> +    ok(hr==0, "IRunningObjectTable_Register hr=%08x %p %08x %p %p %d\n", (int) hr,
> +		pprot, (int) grflags, lpEM1, pmk6, (int) pdwReg2);
> +
> +    IEnumMoniker *spEM=NULL;
> +    hr = IRunningObjectTable_EnumRunning(pprot, &spEM);
> +    todo_wine { ok(hr==0, "IRunningObjectTable_EnumRunning hr=%08x\n", (int) hr); }
> +    // Now see if the we get the same moniker after querying the ROT
> +    IMoniker *pmk61=NULL;
> +    eaten=0;
> +    hr = MkParseDisplayName(pbc, wszDisplayName6, &eaten, &pmk61);
> +    ok(pmk6==pmk61, "Moniker should be the same address %p != %p\n", pmk6, pmk61); 
> +
> +    IRunningObjectTable_Revoke(pprot,pdwReg1);
> +    IRunningObjectTable_Revoke(pprot,pdwReg2);
> +    IEnumMoniker_Release(spEM);
> +    IEnumMoniker_Release(spEM1);
> +    IRunningObjectTable_Release(pprot);
> +    if  (pmk61!=NULL)
> +        IMoniker_Release(pmk61);
> +    if  (pmk6!=NULL)
> +        IMoniker_Release(pmk6);
> +    if  (pmk5!=NULL)
> +        IMoniker_Release(pmk5);
> +    if  (pmk4!=NULL)
> +        IMoniker_Release(pmk4);
> +    if  (pmk3!=NULL)
> +        IMoniker_Release(pmk3);
> +    if  (pmk2!=NULL)
> +        IMoniker_Release(pmk2);
> +    if  (pmk1!=NULL)
> +        IMoniker_Release(pmk1);
>
> -        IUnknown_Release(object);
> -    }
>     IBindCtx_Release(pbc);
> }

Rob

Implementation of EnumMoniker and Partial impl on MkParseDisplayName

[Jeff Latimer]

Rob, I did not grasp what you meant by this.  The examples I looked at 
to set this up  seem to do the way I coded it.

Jeff

Robert Shearman wrote:
> Jeff Latimer wrote:
> > +    DWORD      runObjTabRegister; /* registration key of the next 
> > registered object */
> > +    DWORD      currentPos;    /* enum position in the 
> > list            */
> > +
> > +} EnumMonikerImpl;
> > +
> > +
> > +/* IEnumMoniker prototype functions : */
> > +/* IUnknown functions*/
> > +static HRESULT WINAPI EnumMonikerImpl_QueryInterface(IEnumMoniker* 
> > iface,REFIID riid,void** ppvObject);
> > +static ULONG   WINAPI EnumMonikerImpl_AddRef(IEnumMoniker* iface);
> > +static ULONG   WINAPI EnumMonikerImpl_Release(IEnumMoniker* iface);
> > +/* IEnumMoniker functions */
> > +static HRESULT WINAPI EnumMonikerImpl_Next(IEnumMoniker* iface, 
> > ULONG celt, IMoniker** rgelt, ULONG* pceltFetched);
> > +static HRESULT WINAPI EnumMonikerImpl_Skip(IEnumMoniker* iface, 
> > ULONG celt);
> > +static HRESULT WINAPI EnumMonikerImpl_Reset(IEnumMoniker* iface);
> > +static HRESULT WINAPI EnumMonikerImpl_Clone(IEnumMoniker* iface, 
> > IEnumMoniker** ppenum);
>
> If you put the vtable after the implementations of the functions then 
> you don't need to declare the prototypes here.

+
> > +/* Virtual function table for the IEnumMoniker class. */
> > +static IEnumMonikerVtbl VT_EnumMonikerImpl =
> > +{
> > +    EnumMonikerImpl_QueryInterface,
> > +    EnumMonikerImpl_AddRef,
> > +    EnumMonikerImpl_Release,
> > +    EnumMonikerImpl_Next,
> > +    EnumMonikerImpl_Skip,
> > +    EnumMonikerImpl_Reset,
> > +    EnumMonikerImpl_Clone
> > +};
> > +
> > /***********************************************************************

Implementation of EnumMoniker and Partial impl on MkParseDisplayName

[Mike McCormack]

Jeff Latimer wrote:
> Rob, I did not grasp what you meant by this.  The examples I looked at 
> to set this up  seem to do the way I coded it.

If you put the vtable at the end of the file after all the functions are 
declared, there should be no need to declare any functions as prototypes.

Mike