[PATCH v5 0/1] MR5373: shell32: Implement FCIDM_SHVIEW_CREATELINK

[Aric Stewart (＠aricstewart)]

Presently the wine file explorer has a create shortcut entry that does nothing. This implements the FCIDM_SHVIEW_CREATELINK command.

This is a patch that was first submitted in 2017 and I unfortunately let the revision request fall unimplemented.

--
  v5: shell32: Implement FCIDM_SHVIEW_CREATELINK
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5373

[PATCH v5 1/1] shell32: Implement FCIDM_SHVIEW_CREATELINK

[Aric Stewart]

From: Aric Stewart <aric(a)codeweavers.com>

---
 dlls/shell32/shlview_cmenu.c | 88 ++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/dlls/shell32/shlview_cmenu.c b/dlls/shell32/shlview_cmenu.c
index cac33c39af7..9f0d9799491 100644
--- a/dlls/shell32/shlview_cmenu.c
+++ b/dlls/shell32/shlview_cmenu.c
@@ -946,6 +946,90 @@ static void DoOpenProperties(ContextMenu *This, HWND hwnd)
 	    FIXME("No property pages found.\n");
 }
 
+
+static void DoCreateLink(ContextMenu *This)
+{
+    IShellLinkW* shelllink;
+    IPersistFile* persistfile;
+    WCHAR wszFilename[MAX_PATH];
+    WCHAR wszLinkFilename[MAX_PATH];
+    WCHAR root[MAX_PATH];
+    WCHAR lnkfile[MAX_PATH];
+    LPITEMIDLIST full_pidl;
+    HRESULT hr;
+    int length;
+    int counter = 1;
+
+    static const WCHAR *shortcutW = L"Shortcut";
+    static const WCHAR *lnkW = L".lnk";
+
+    if (FAILED(IShellLink_Constructor(NULL, &IID_IShellLinkW, (LPVOID*)&shelllink)))
+    {
+        ERR("couldn't create ShellLink object\n");
+        return;
+    }
+    full_pidl = ILCombine(This->pidl, This->apidl[0]);
+    if (!full_pidl)
+    {
+        ERR("Out of Memory\n");
+        return;
+    }
+
+    hr = IShellLinkW_SetIDList(shelllink, full_pidl);
+    ILFree(full_pidl);
+    if (FAILED(hr)) {
+        ERR("SetIDList failed\n");
+        return;
+    }
+    _ILSimpleGetTextW(This->apidl[0], (LPVOID)wszFilename, MAX_PATH);
+    length = lstrlenW(wszFilename) + lstrlenW(shortcutW);
+    if (length > MAX_PATH) {
+        ERR("Path maximum length exceeded (%i>%i)\n", length, MAX_PATH);
+        IShellLinkW_Release(shelllink);
+        return;
+    }
+
+    SHGetPathFromIDListW(This->pidl, root);
+    if (PathIsSystemFolderW(root, 0)) {
+        if (!SHGetSpecialFolderPathW(HWND_DESKTOP, root, CSIDL_DESKTOP, FALSE)) {
+            ERR("Target folder is System folder and Failed to get Desktop directory\n");
+            IShellLinkW_Release(shelllink);
+            return;
+        }
+    }
+
+    do {
+        if (counter == 1) {
+            wsprintfW(wszLinkFilename, L"%s - %s", wszFilename, shortcutW);
+        } else {
+            wsprintfW(wszLinkFilename, L"%s - %s (%u)", wszFilename, shortcutW, counter);
+        }
+
+        length = lstrlenW(root) + lstrlenW(wszLinkFilename) + lstrlenW(lnkW);
+        if (length > MAX_PATH) {
+            ERR("Path maximum length exceeded (%i>%i)\n", length, MAX_PATH);
+            IShellLinkW_Release(shelllink);
+            return;
+        }
+        PathCombineW(lnkfile, root, wszLinkFilename);
+        PathAddExtensionW(lnkfile, lnkW);
+        counter++;
+    } while (PathFileExistsW(lnkfile));
+
+    if (!SUCCEEDED(IShellLinkW_QueryInterface(shelllink, &IID_IPersistFile, (LPVOID*)&persistfile)))
+    {
+        ERR("couldn't get IPersistFile interface\n");
+        IShellLinkW_Release(shelllink);
+        return;
+    }
+    TRACE("writing link to %s\n", wine_dbgstr_w(lnkfile));
+    if (!SUCCEEDED(IPersistFile_Save(persistfile, lnkfile, FALSE)))
+        ERR("couldn't write link to %s\n", wine_dbgstr_w(lnkfile));
+
+    IPersistFile_Release(persistfile);
+    IShellLinkW_Release(shelllink);
+}
+
 static HRESULT WINAPI ItemMenu_InvokeCommand(
 	IContextMenu3 *iface,
 	LPCMINVOKECOMMANDINFO lpcmi)
@@ -1014,6 +1098,10 @@ static HRESULT WINAPI ItemMenu_InvokeCommand(
             TRACE("Verb FCIDM_SHVIEW_PROPERTIES\n");
             DoOpenProperties(This, lpcmi->hwnd);
             break;
+        case FCIDM_SHVIEW_CREATELINK:
+            TRACE("Verb FCIDM_SHVIEW_CREATELINK\n");
+            DoCreateLink(This);
+            break;
         default:
             FIXME("Unhandled verb %#x.\n", id);
             return E_INVALIDARG;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/5373

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/shell32/shlview_cmenu.c:
> FIXME("No property pages found.\n");
>  }
>
> +
> +static void DoCreateLink(ContextMenu *This)

Let's call this `do_create_link()`

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_71730

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/shell32/shlview_cmenu.c:
> FIXME("No property pages found.\n");
>  }
>
> +
> +static void DoCreateLink(ContextMenu *This)
> +{
> +    IShellLinkW* shelllink;
> +    IPersistFile* persistfile;
> +    WCHAR wszFilename[MAX_PATH];
> +    WCHAR wszLinkFilename[MAX_PATH];
> +    WCHAR root[MAX_PATH];
> +    WCHAR lnkfile[MAX_PATH];

Can we dynamically allocate (at least some of) these so we don't have to have `MAX_PATH` limits?

Also can we drop the `wsz` prefixes and snake case things?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_71731

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/shell32/shlview_cmenu.c:
> FIXME("No property pages found.\n");
>  }
>
> +
> +static void DoCreateLink(ContextMenu *This)
> +{
> +    IShellLinkW* shelllink;
> +    IPersistFile* persistfile;
> +    WCHAR wszFilename[MAX_PATH];
> +    WCHAR wszLinkFilename[MAX_PATH];
> +    WCHAR root[MAX_PATH];
> +    WCHAR lnkfile[MAX_PATH];
> +    LPITEMIDLIST full_pidl;

`ITEMIDLIST *full_pidl;`

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_71732

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/shell32/shlview_cmenu.c:
> +
> +    if (FAILED(IShellLink_Constructor(NULL, &IID_IShellLinkW, (LPVOID*)&shelllink)))
> +    {
> +        ERR("couldn't create ShellLink object\n");
> +        return;
> +    }
> +    full_pidl = ILCombine(This->pidl, This->apidl[0]);
> +    if (!full_pidl)
> +    {
> +        ERR("Out of Memory\n");
> +        return;
> +    }
> +
> +    hr = IShellLinkW_SetIDList(shelllink, full_pidl);
> +    ILFree(full_pidl);
> +    if (FAILED(hr)) {
> Let's not use K&R braces.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_71733

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/shell32/shlview_cmenu.c:
> +    }
> +    full_pidl = ILCombine(This->pidl, This->apidl[0]);
> +    if (!full_pidl)
> +    {
> +        ERR("Out of Memory\n");
> +        return;
> +    }
> +
> +    hr = IShellLinkW_SetIDList(shelllink, full_pidl);
> +    ILFree(full_pidl);
> +    if (FAILED(hr)) {
> +        ERR("SetIDList failed\n");
> +        return;
> +    }
> +    _ILSimpleGetTextW(This->apidl[0], (LPVOID)wszFilename, MAX_PATH);
> +    length = lstrlenW(wszFilename) + lstrlenW(shortcutW);
> `wcslen()`, but note that this check could lead to `wszLinkFilename` overflowing (howevever if we switch to dynamic allocation this will likely change anyway).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_71734

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/shell32/shlview_cmenu.c:
> +        if (counter == 1) {
> +            wsprintfW(wszLinkFilename, L"%s - %s", wszFilename, shortcutW);
> +        } else {
> +            wsprintfW(wszLinkFilename, L"%s - %s (%u)", wszFilename, shortcutW, counter);
> +        }
> +
> +        length = lstrlenW(root) + lstrlenW(wszLinkFilename) + lstrlenW(lnkW);
> +        if (length > MAX_PATH) {
> +            ERR("Path maximum length exceeded (%i>%i)\n", length, MAX_PATH);
> +            IShellLinkW_Release(shelllink);
> +            return;
> +        }
> +        PathCombineW(lnkfile, root, wszLinkFilename);
> +        PathAddExtensionW(lnkfile, lnkW);
> +        counter++;
> +    } while (PathFileExistsW(lnkfile));
> This will be racey.  However I wonder if shelllink's `IPersistFile_Save` should do this instead?  A test would be good.  If not, then I'd suggest actually trying to create the file in the loop.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_71735

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/shell32/shlview_cmenu.c:
> +        IShellLinkW_Release(shelllink);
> +        return;
> +    }
> +
> +    SHGetPathFromIDListW(This->pidl, root);
> +    if (PathIsSystemFolderW(root, 0)) {
> +        if (!SHGetSpecialFolderPathW(HWND_DESKTOP, root, CSIDL_DESKTOP, FALSE)) {
> +            ERR("Target folder is System folder and Failed to get Desktop directory\n");
> +            IShellLinkW_Release(shelllink);
> +            return;
> +        }
> +    }
> +
> +    do {
> +        if (counter == 1) {
> +            wsprintfW(wszLinkFilename, L"%s - %s", wszFilename, shortcutW);
> Couldn't the `".lnk"` be appended here?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_71736

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/shell32/shlview_cmenu.c:
> +    _ILSimpleGetTextW(This->apidl[0], (LPVOID)wszFilename, MAX_PATH);
> +    length = lstrlenW(wszFilename) + lstrlenW(shortcutW);
> +    if (length > MAX_PATH) {
> +        ERR("Path maximum length exceeded (%i>%i)\n", length, MAX_PATH);
> +        IShellLinkW_Release(shelllink);
> +        return;
> +    }
> +
> +    SHGetPathFromIDListW(This->pidl, root);
> +    if (PathIsSystemFolderW(root, 0)) {
> +        if (!SHGetSpecialFolderPathW(HWND_DESKTOP, root, CSIDL_DESKTOP, FALSE)) {
> +            ERR("Target folder is System folder and Failed to get Desktop directory\n");
> +            IShellLinkW_Release(shelllink);
> +            return;
> +        }
> +    }
> I'd suggest leaving this out for now.  We'd want to test whether `PathIsSystemFolderW()` is the correct way to determine this or whether the test should be on the pidl.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5373#note_71737