Hi Jeff,
Jeff Latimer wrote:
> This is the first installment of the implementation of
> MkParseDisplayName. I would like feed back on style and usage.
>
> I think the implementation of the EnumMoniker interface looks ok.
> Comments appreciated.
I like that you've taken the time to write a test case! I don't know
alot about monikers, but seeing that you've tested it gives me confidence...
The following comments are a crash course in Wine programming...
> +/***********************************************************************
> + * EnmumMoniker_Next
> + */
> +HRESULT WINAPI EnumMonikerImpl_Next(IEnumMoniker* iface, ULONG celt, IMoniker** rgelt, ULONG * pceltFetched)
> +{
> +// DWORD i;
> +// ULONG ref;
We avoid C++ comments in Wine code to keep compatability with older
compilers.
> + EnumMonikerImpl *This = (EnumMonikerImpl *)iface;
> + TRACE("(%p) currentPos %d Tablastindx %d\n",This, (int)This->currentPos, (int)This->TabLastIndx);
> + ULONG i;
You declared "i" above, but commented it out. Try avoid putting dead
code in comments. If code isn't compiled, it will become stale and
confuse people.
> + rc = CoGetMalloc(1, (LPMALLOC *) &ppMalloc);
> 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
That's alot of casts. I don't think you really need that many, do you?
> + /* 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;
Better to avoid tabs... and too many brackets. A space or two extra in
the for( i=0; (This->... might make things look better.
> + if (szUserName[char_cnt] == '/' ||
> + szUserName[char_cnt] == '?' ||
> + szUserName[char_cnt] == '<' ||
> + szUserName[char_cnt] == '>' ||
> + szUserName[char_cnt] == '*' ||
> + szUserName[char_cnt] == '|' ||
> + szUserName[char_cnt] == ':')
Maybe that can be shortened to :
if (strchr("/?<>*|:",szUserName[char_cnt]))
> + todo_wine { ok(hr==0x800401e4, "MkParseDisplayName - Progid not implemented hr=%08x\n", (int) hr); }
0x800401e4 is defined as MK_E_SYNTAX. It would be clearer to use the
define rather than a number.
%08lx is probably what you want instead of casting hr to (int) to get
rid of the printf parameter type warning.
> +
> + 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};
Again, for compatability, we avoid inline declarations, even though gcc
can deal with them fine.
Mike