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.