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
