Re: [PATCH 1/5] shell32: Implement loading of context menu shell extensions (try 3)

[Nikolay Sivov]

On 8/21/2011 19:20, Jay Yang wrote:
> The first two patches in the sequence relate to context menus and the last three to property sheets,
> but some of the changes in the last three depend on a function I created in the second patch so I've
> sent them as a sequence.
>
> Changes from last time (for this file):
> cleaned up the code.
>
> ---
>   dlls/shell32/shlmenu.c |  170 ++++++++++++++++++++++++++++++++++++++++++++++-
>   1 files changed, 166 insertions(+), 4 deletions(-)
> +    HKEY *new_keys=NULL;
>
> +    new_keys = HeapAlloc(GetProcessHeap(),0,sizeof(HKEY)*cKeys);
> No need to initialize that explicitly.
> +        ULONG status = RegOpenKeyExW(aKeys[i],cmenu_handler_key,0,KEY_READ,&new_key);
> +        if(status==ERROR_SUCCESS)
> This call return LSTATUS which is signed LONG. A common way to test 
> return value is to use (!RegOpen*()).
> +    CLSID *clsid_table=NULL;
> ...
> +    clsid_table = HeapAlloc(GetProcessHeap(),HEAP_ZERO_MEMORY,sizeof(CLSID)*handler_count);
> Same here, no need to NULL it.
> +                static const CLSID zero_clsid = {0};
> +                CLSID *curr_clsid = clsid_table+j;
> +                if(IsEqualCLSID(curr_clsid,&zero_clsid))
> There's CLSID_NULL for that.
> +            TRACE("Loading shell extension with clsid %s\n",shdebugstr_guid(&id));
> I'm not sure shdebugstr_guid() is supposed to know shell extension names.
> +            if(new_keys[i])
> +                RegCloseKey(new_keys[i]);
> This null check is redundant.