[PATCH] shlwapi: initial implement of SHAutoComplete
Nikolay Sivov
nsivov at codeweavers.com
Sun May 23 15:22:41 CDT 2010
On 5/23/2010 05:09, Hirofumi Katayama wrote:
> See attachment.
>
Hi, some comments here.
> +/****************************************************************************/
> +
> +static const WCHAR szMRUList[] = L"MRUList";
> +static const WCHAR szRunMRU[] =
> + L"Software\\Microsoft\\Windows\\CurrentVersion\\Explorer\\RunMRU";
> +
>
Don't use this L"" specifier for strings, use array initialization style
instead.
> +void *my_malloc(UINT size)
> +{
> + return HeapAlloc(GetProcessHeap(), 0, size);
> +}
> +
> +void *my_realloc(void *p, UINT size)
> +{
> + if (p == NULL)
> + return my_malloc(size);
> + return HeapReAlloc(GetProcessHeap(), 0, p, size);
> +}
> +
> +void my_free(void *p)
> +{
> + HeapFree(GetProcessHeap(), 0, p);
> +}
> +
>
This isn't very useful, I think it's not a pain to use direct calls for
this file.
> +#define malloc my_malloc
> +#define realloc my_realloc
> +#define free my_free
> +#define _wcsdup my__wcsdup
>
Don't do that.
> +
> +STDMETHODIMP IEnumString_fnQueryInterface(IEnumString *pEnumString, REFIID riid, void** ppvObject);
> +STDMETHODIMP_(ULONG) IEnumString_fnAddRef(IEnumString *pEnumString);
> +STDMETHODIMP_(ULONG) IEnumString_fnRelease(IEnumString *pEnumString);
> +STDMETHODIMP IEnumString_fnNext(IEnumString *pEnumString, ULONG celt, LPOLESTR *rgelt, ULONG *pceltFetched);
> +STDMETHODIMP IEnumString_fnSkip(IEnumString *pEnumString, ULONG celt);
> +STDMETHODIMP IEnumString_fnReset(IEnumString *pEnumString);
> +STDMETHODIMP IEnumString_fnClone(IEnumString *pEnumString, IEnumString **ppenum);
>
STDMETHODIMP isn't used in Wine code, use explicit WINAPI please. Also
forward declaration for methods isn't used, usually a vtable is
initialized just after all methods bodies.
> +typedef struct
> +{
> + IEnumStringVtbl *lpVtbl;
> + LONG m_lRef;
> + IMalloc * m_pMalloc;
> + INT m_i;
> + INT m_c;
> + LPWSTR * m_strings;
> +} EnumStringImpl;
>
Please avoid this MFC style like m_* for members, and use more
descriptive names.
> +EnumStringImpl *IEnumString_construct(VOID)
> +{
> + EnumStringImpl *pEnumString;
> +
> + pEnumString = (EnumStringImpl *) malloc(sizeof(EnumStringImpl));
> + if (pEnumString != NULL)
> + {
> + pEnumString->lpVtbl =&EnumStringVtbl;
> + pEnumString->m_lRef = 0;
> + pEnumString->m_pMalloc = NULL;
> + SHGetMalloc(&pEnumString->m_pMalloc);
> + pEnumString->m_i = 0;
> + pEnumString->m_strings = GetMRUStrings(&pEnumString->m_c);
> + }
> + return pEnumString;
> +}
>
This is wrong. Initial refcount should be 1.
> +EnumStringImpl *IEnumString_construct(VOID)
>
> +
> +VOID IEnumString_destruct(EnumStringImpl *pEnumString)
>
Should be static both, and methods too of course.
> + pEnumString->lpVtbl->AddRef(pEnumString);
>
No need for that, use defined macros to call methods from vtables.
> +STDMETHODIMP_(ULONG) IEnumString_fnAddRef(IEnumString *pEnumString)
> +{
> + EnumStringImpl *this = (EnumStringImpl *) pEnumString;
> + this->m_lRef++;
> + return (ULONG) this->m_lRef;
> +}
>
You should use interlocked increment here (same for _Release()).
> + pProp->fnOldWndProc = (WNDPROC) SetWindowLongPtr(hwndEdit, GWLP_WNDPROC, (LONG_PTR) AutoCompleteEditWndProc);
>
Did you ever build it? I think we have a blocking things for using
implicit A/W calls.
More information about the wine-devel
mailing list