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