[PATCH 3/5] shell32: Implement SHOpenPropSheet

[Nikolay Sivov]

On 8/21/2011 19:20, Jay Yang wrote:
> ---
>   dlls/shell32/shell32.spec |    3 ++
>   dlls/shell32/shellord.c   |   88 +++++++++++++++++++++++++++++++++++++++++++++
>   include/shlobj.h          |    4 ++
>   3 files changed, 95 insertions(+), 0 deletions(-)
It's not clear for me why you placed this in shellord.c, it's supposed 
to contain exported by ordinal calls only, as I understand it. If 
Julliard approves new file for propsheets then you should place this in 
it. IMO it's ok to place all of this in shlmenu.c for example.

> +BOOL WINAPI SHOpenPropSheetA(LPCSTR pszCaption, HKEY *ahkey, UINT ckeys, const CLSID *pclsidDef, IDataObject *pdtobj, IShellBrowser *psb, LPCSTR pStartPage)
> +{
> +    BOOL ret;
> +    LPWSTR wszCaption,wszStartPage;
> +    UINT length;
> +    length = MultiByteToWideChar(CP_ACP, 0, pszCaption, -1, NULL, 0);
> +    wszCaption = HeapAlloc(GetProcessHeap(),0,sizeof(WCHAR)*length);
> +    length = MultiByteToWideChar(CP_ACP, 0, pszCaption, -1, wszCaption, length);
> +    length = MultiByteToWideChar(CP_ACP, 0, pStartPage, -1, NULL, 0);
> +    wszStartPage = HeapAlloc(GetProcessHeap(),0,sizeof(WCHAR)*length);
> +    length = MultiByteToWideChar(CP_ACP, 0, pStartPage, -1, wszStartPage, length);
> +    ret = SHOpenPropSheetW(wszCaption,ahkey,ckeys,pclsidDef,pdtobj,psb,wszStartPage);
> +    HeapFree(GetProcessHeap(),0,wszCaption);
> +    HeapFree(GetProcessHeap(),0,wszStartPage);
> +    return ret;
> +}
Hmm, actually there's HEAP_strdupAtoW() for that (you need to make in 
non-static though).

> +	ZeroMemory(&psh, sizeof(PROPSHEETHEADERW));
> +	psh.dwSize = sizeof (PROPSHEETHEADERW);
> +	psh.hwndParent = NULL;
> +	psh.dwFlags = PSH_PROPTITLE;
> +    if(pStartPage)
> +    {
> +        psh.dwFlags |= PSH_USEPSTARTPAGE;
> +        psh.u2.pStartPage = pStartPage;
> +    }
Please don't mix tabs with spaces, use 4 spaces everywhere.

> +    if(pszCaption)
> +        psh.pszCaption = pszCaption;
> +    else
> +        psh.pszCaption = empty_str;
Is it really needed? I mean pszCaption == NULL is not the same as empty 
string in this context?

> +    IShellPropSheetExt *def_ext;
> +    IShellExtInit *init_iface;
> +    PROPSHEETHEADERW psh;
> +	HPROPSHEETPAGE pages[99];
It's not clear again where this size comes from, just enough for anyone? 
Also I think it's better to reduce scope as much as possible, def_ext 
doesn't need to be global for example.