Christian Costa wrote:
>
> Hi,
>
> This patch adds implementation for the IFilterMapper interface.
Nice. I have few comments (see below).
> However filters registered with this interface require some work in
> devenum to
> be seen from application. This will be done when time permits.
Ok. I had assumed devenum was complete, but obviously not. Does it spit out
any fixme's?
...
> @@ -879,7 +893,7 @@
> if (SUCCEEDED(hrSub))
> hrSub =
> ICreateDevEnum_CreateClassEnumerator(pCreateDevEnum, &clsidCat,
> &pEnum, 0);
>
> - if (SUCCEEDED(hrSub))
> + if (SUCCEEDED(hrSub) && (hrSub == S_OK))
You can remove the SUCCEEDED(hrSub) completely here.
...
> + if (SUCCEEDED(hrSub))
> + {
> + len = (strlenW((WCHAR*)&V_UNION(&var, bstrVal))+1) *
> sizeof(WCHAR);
> + if (!(regfilters[idx].Name = CoTaskMemAlloc(len*2)))
> + hr = E_OUTOFMEMORY;
> + }
> +
> + if (SUCCEEDED(hrSub))
> + {
> + memcpy(regfilters[idx].Name, &V_UNION(&var, bstrVal), len);
> + regfilters[idx].Clsid = clsid;
> + idx++;
> + }
Indentation messed up a bit here and many other places.
...
> + if (cFetched > 0)
> + {
> + for(i = 0; i < cFetched; i++) {
> + /* The string in the REGFILTER structure must be
> allocated in the same block as the REGFILTER structure itself */
> + ppRegFilter[i] =
> (REGFILTER*)CoTaskMemAlloc(sizeof(REGFILTER)+(strlenW(This->RegFil
> ters[i].Name)+1)*sizeof(WCHAR));
> + if (!ppRegFilter[i]) {
> + while(i) {
> + CoTaskMemFree(ppRegFilter[--i]);
> + ppRegFilter[i] = NULL;
> + }
> + return E_OUTOFMEMORY;
> + }
> + ppRegFilter[i]->Clsid = This->RegFilters[i].Clsid;
> + ppRegFilter[i]->Name =
> (WCHAR*)((char*)ppRegFilter[i]+sizeof(REGFILTER));
> + CopyMemory(ppRegFilter[i]->Name,
> This->RegFilters[i].Name,
> (strlenW(This->RegFilters[i].Name)+1)*sizeof(WCHAR));
> + }
Please choose one style of bracket placement and stick to it (this occurs in
a few places too).
Other than these things, it looks fine.
Rob
