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