[PATCH] shell32: fix two bugs in IQueryAssocations

[Theodore Dubois]

This is sort of a work in progress. I just wanted to see what Marvin had to say about it. Please ignore for now.

~Theodore

> On Feb 13, 2016, at 3:34 PM, Nikolay Sivov <bunglehead at gmail.com> wrote:
> 
> On 14.02.2016 2:19, Theodore Dubois wrote:
>> Basically, returns the correct value for
>> ASSOCSTR_FRIENDLYDOCNAME and ASSOCSTR_DEFAULTICON
>> when passed a ProgID instead of a file extension.
>> 
>> Also adds some tests to make sure it works.
>> 
>> Signed-off-by: Theodore Dubois <tblodt at icloud.com>
>> ---
>> dlls/shell32/assoc.c       | 146 +++++++++++++++++++++++++--------------------
>> dlls/shell32/tests/assoc.c |  71 +++++++++++++++++++++-
>> 2 files changed, 151 insertions(+), 66 deletions(-)
>> 
>> diff --git a/dlls/shell32/assoc.c b/dlls/shell32/assoc.c
>> index 90d00a8..2667ffe 100644
>> --- a/dlls/shell32/assoc.c
>> +++ b/dlls/shell32/assoc.c
>> @@ -63,6 +63,7 @@ typedef struct
>>  LONG ref;
>>  HKEY hkeySource;
>>  HKEY hkeyProgID;
>> +  LPWSTR pszAssocName;
>> } IQueryAssociationsImpl;
> 
> Why do you need this? It's not used anywhere as far as I can tell.
> 
>> 
>> typedef struct
>> @@ -88,6 +89,8 @@ static inline struct enumassochandlers *impl_from_IEnumAssocHandlers(IEnumAssocH
>>  return CONTAINING_RECORD(iface, struct enumassochandlers, IEnumAssocHandlers_iface);
>> }
>> 
>> +static HRESULT ASSOC_GetValue(HKEY hkey, const WCHAR *name, void **data, DWORD *data_size);
>> +
>> /**************************************************************************
>> *  IQueryAssociations_QueryInterface
>> *
>> @@ -152,6 +155,7 @@ static ULONG WINAPI IQueryAssociations_fnRelease(IQueryAssociations *iface)
>>    TRACE("Destroying IQueryAssociations (%p)\n", This);
>>    RegCloseKey(This->hkeySource);
>>    RegCloseKey(This->hkeyProgID);
>> +    HeapFree(GetProcessHeap(), 0, (LPVOID)This->pszAssocName);
>>    SHFree(This);
>>  }
> 
> No need for a cast.
> 
>> 
>> @@ -186,20 +190,30 @@ static HRESULT WINAPI IQueryAssociations_fnInit(
>>    LONG ret;
>> 
>>    TRACE("(%p)->(%d,%s,%p,%p)\n", iface,
>> -                                    cfFlags,
>> -                                    debugstr_w(pszAssoc),
>> -                                    hkeyProgid,
>> -                                    hWnd);
>> +                                   cfFlags,
>> +                                   debugstr_w(pszAssoc),
>> +                                   hkeyProgid,
>> +                                   hWnd);
> 
> No need for that either.
> 
>> +
>> +        This->pszAssocName = HeapAlloc(GetProcessHeap(), 0, (strlenW(pszAssoc) + 10001) * sizeof(WCHAR));
> 
> Seriously?
> 
>> -  *data = HeapAlloc(GetProcessHeap(), 0, size);
>> +  *data = HeapAlloc(GetProcessHeap(), 0, size + 100);
> 
> What is that?
> 
>>  if (!*data)
>>    return E_OUTOFMEMORY;
>>  ret = RegQueryValueExW(hkey, name, 0, NULL, (LPBYTE)*data, &size);
>> @@ -507,33 +550,16 @@ static HRESULT WINAPI IQueryAssociations_fnGetString(
>> 
>>    case ASSOCSTR_FRIENDLYDOCNAME:
>>    {
>> -      WCHAR *pszFileType;
>> -      DWORD ret;
>> -      DWORD size;
>> +      WCHAR *docName;
>> +
>> +      if (This->hkeyProgID == NULL) {
>> +
>> +      }
> 
> And that?
> 
> 
>> diff --git a/dlls/shell32/tests/assoc.c b/dlls/shell32/tests/assoc.c
>> index 8aa2535..1ba4261 100644
>> --- a/dlls/shell32/tests/assoc.c
>> +++ b/dlls/shell32/tests/assoc.c
>> @@ -91,6 +91,8 @@ struct assoc_getstring_test
>> };
>> 
>> static const WCHAR httpW[] = {'h','t','t','p',0};
>> +static const WCHAR dot_urlW[] = {'.','u','r','l',0};
>> +static const WCHAR InternetShortcutW[] = {'I','n','t','e','r','n','e','t','S','h','o','r','t','c','u','t',0};
>> static const WCHAR badW[] = {'b','a','d','b','a','d',0};
>> 
>> static struct assoc_getstring_test getstring_tests[] =
>> @@ -135,7 +137,7 @@ static void test_IQueryAssociations_GetString(void)
>>        else
>>        {
>>            ok(hr == ptr->hr, "%d: GetString failed, 0x%08x\n", i, hr);
>> -            ok(len > ptr->len, "%d: got needed length %d\n", i, len);
>> +            ok(len >= ptr->len, "%d: got needed length %d\n", i, len);
>>        }
> 
> Why '>' was enough before?
> 
>> +    getstring_test(0, NULL, test_progid_key, ASSOCSTR_DEFAULTICON, NULL, S_OK, test_iconW);
>> +
> 
> If it's only called with 0 flags, there's no need in this argument.
> 
>> +    //RegDeleteKeyExW(HKEY_CLASSES_ROOT, test_extensionW, KEY_WOW64_32KEY, 0);
>> +    //RegDeleteKeyExW(HKEY_CLASSES_ROOT, test_progidW, KEY_WOW64_32KEY, 0);
>> +}
> 
> Leftovers?
> 
>> +
>> static void test_IQueryAssociations_Init(void)
>> {
>>    IQueryAssociations *assoc;
>> @@ -229,6 +297,7 @@ START_TEST(assoc)
>>    if (hr == S_OK)
>>    {
>>        test_IQueryAssociations_QueryInterface();
>> +        test_IQueryAssociations_GetString_results();
>>        test_IQueryAssociations_GetString();
>>        test_IQueryAssociations_Init();
> 
> Please merge this with existing *_GetString().