[v2] shell32: fix two bugs in IQueryAssocations

Nikolay Sivov bunglehead at gmail.com
Sun Feb 7 18:38:10 CST 2016


On 08.02.2016 3:07, Theodore Dubois wrote:
> 
>> On Feb 7, 2016, at 4:25 AM, Nikolay Sivov <bunglehead at gmail.com> wrote:
>>
>> Why do you need this?
> 
> So I can assert in Init.

That's not necessary.

> 
>>
>>>             This->hkeyProgID = This->hkeySource;
>>
>> I know it's not something you introduced, but it's dirty and should be
>> fixed, because it leads to double free on a same handle.
> 
> I’ll fix that in the next version.
> 
>> Basically you remove everything, and tests don't show that you should do
>> that. For example looking at Win7 registry I see HKCR\.exe with default
>> value of "exefile", then default value for HKCR\exefile is Application
>> (and also there is a FriendlyTypeName key that's most likely localized).
>> Current code will return "Application" and that seems like a better
>> candidate for a friendly name.
> 
> In that case, hkeyProgID would refer to HKCR\exefile, and then I return the
> default value from that key which is “Application”. It used to start with hkeySource 
> (would point to HKCR\.exe) and do some digging, but my code does that
work in Init.

> +        if (*pszAssoc == '.')
> +        {
> +            /* for a file extension, the progid is the default value */
> +            hr = ASSOC_GetValue(This->hkeySource, NULL, (void**)&progId, NULL);
> +            if (FAILED(hr))
> +                return hr;
> +        }

What happens if default value is not set?

> +        else /* if (*pszAssoc == '{') */
> +        {
> +            assert(*pszAssoc == '{');
> +
> +            HKEY progIdKey;
> +            /* for a clsid, the progid is the default value of the ProgID subkey */
> +            ret = RegOpenKeyExW(This->hkeySource,
> +                                szProgID,
> +                                0,
> +                                KEY_READ,
> +                                &progIdKey);
> +            if (ret != ERROR_SUCCESS)
> +                return HRESULT_FROM_WIN32(ret);
> +            hr = ASSOC_GetValue(progIdKey, NULL, (void**)&progId, NULL);
> +            if (FAILED(hr))
> +                return hr;
> +            RegCloseKey(progIdKey);
> +        }

Now Init() will fail on missing progid subkey, and it didn't before.
Also this introduces leaks.

> 
>> A convincing test should create a custom extension key, and test all
>> possible combinations of empty default values, missing keys with default
>> value name etc.
> 
> Yes, more tests is always good. I’ll try and figure out how to test things on Windows and add some more tests in the next version.
> 
> ~Theodore
> 




More information about the wine-devel mailing list