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.