Re: [PATCH v3] shell32: Implement SHCreateItemInKnownFolder.

[Huw Davies]

On Tue, Aug 08, 2017 at 03:04:21PM +0800, Jactry Zeng wrote:
> diff --git a/dlls/shell32/shellitem.c b/dlls/shell32/shellitem.c
> index 1e19f7c72e..0bfb4a7f6f 100644
> --- a/dlls/shell32/shellitem.c
> +++ b/dlls/shell32/shellitem.c
> @@ -662,6 +662,67 @@ HRESULT WINAPI SHCreateItemFromIDList(PCIDLIST_ABSOLUTE pidl, REFIID riid, void
>      return ret;
>  }
>
> +HRESULT WINAPI SHCreateItemInKnownFolder(REFKNOWNFOLDERID rfid, DWORD flags,
> -
>                                     LPWSTR filename, REFIID riid, void **ppv)
>
> +{
> - HRESULT hr;
> - WCHAR *pathW = NULL;
> - IShellFolder *desktop = NULL, *folder = NULL;
> - LPITEMIDLIST pidl_folder = NULL, pidl = NULL;
> -
> - TRACE("%p, %x, %s, %s, %p\n", rfid, flags, wine_dbgstr_w(filename),
> -
>      debugstr_guid(riid), ppv);
> -
> - if(!rfid || !ppv)
> -
>    return E_INVALIDARG;
> -
> - if(flags)
> -
>    FIXME("flags 0x%08x not supported\n", flags);
> -
> - *ppv = NULL;
> - hr = SHGetKnownFolderIDList(rfid, 0, NULL, &pidl_folder);

flags should just be passed as the second parameter here and then you
can ditch the FIXME().
> - if(hr != S_OK)
> -
>    goto cleanup;
> -
> - hr = SHGetDesktopFolder(&desktop);
> - if(hr != S_OK)
> -
>    goto cleanup;
> -
> - if(!IsEqualIID(rfid, &FOLDERID_Desktop))
> - {
> -
>    hr = IShellFolder_BindToObject(desktop, pidl_folder, NULL, &IID_IShellFolder,
> -
>                                   (void**)&folder);
> -
>    if(hr != S_OK)
> -
>        goto cleanup;
> - }
> -
> - if(filename)
> - {
> -
>    hr = IShellFolder_ParseDisplayName(folder ? folder : desktop, NULL, NULL,
> -
>                                       filename, NULL, &pidl, NULL);
> -
>    if(hr != S_OK)
> -
>        goto cleanup;
> -
>    hr = SHCreateItemFromIDList(pidl, riid, ppv);
> -
>    if(hr != S_OK)
> -
>        goto cleanup;
> - }
> - else
> -
>    hr = SHCreateItemFromIDList(pidl_folder, riid, ppv);

Thinking about this some more, how about implementing
SHCreateItemFromRelativeName() first?  Then this function becomes
something like:

SHGetKnownFolderIDList( rfid, flags, NULL, &pidl );
   SHCreateItemFromIDList( pidl, &IID_IShellItem, &parent );
   if (name)
      SHCreateItemFromRelativeName( parent, name, NULL, iid, ppv );
   else
      IShellItem_QueryInterface( parent, iid, ppv );
   IShellItem_Release( parent );
   ILFree( pidl );
> diff --git a/dlls/shell32/tests/shlfolder.c b/dlls/shell32/tests/shlfolder.c
> index 9e7127dfea..461f8cea95 100644
> --- a/dlls/shell32/tests/shlfolder.c
> +++ b/dlls/shell32/tests/shlfolder.c
> - /* SHCreateItemInKnownFolder */
> - if(pSHCreateItemInKnownFolder || pSHGetKnownFolderPath)

I think you mean && here (although in practice it doesn't matter).
> -
>    if(0)
> -
>    {
> -
>        /* crashes on Windows */
> -
>        pSHCreateItemInKnownFolder(NULL, 0, NULL, NULL, NULL);
> -
>        pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, NULL, NULL, NULL);
> -
>        pSHCreateItemInKnownFolder(&FOLDERID_Desktop, 0, testfileW, &IID_IShellItem, NULL);
> -
>    }

I'm not a big fan of this sort of thing, just leave it out.

Huw.