[PATCH] shell32: IPersistFile::Save(NULL) should use previously stored file name.

[Dmitry Timoshkov]

This is a resend, previous version didn't get much attention.

Signed-off-by: Dmitry Timoshkov 
---
 dlls/shell32/shelllink.c       | 14 ++++++++++----
 dlls/shell32/tests/shelllink.c | 10 ++++++++++
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/dlls/shell32/shelllink.c b/dlls/shell32/shelllink.c
index 5b044d4be1..23c5935a7b 100644
--- a/dlls/shell32/shelllink.c
+++ b/dlls/shell32/shelllink.c
@@ -367,7 +367,10 @@ static HRESULT WINAPI IPersistFile_fnSave(IPersistFile* iface, LPCOLESTR pszFile
     TRACE("(%p)->(%s)\n",This,debugstr_w(pszFileName));

if (!pszFileName)
-        return E_FAIL;
+    {
+        pszFileName = This->filepath;
+        fRemember = FALSE;
+    }

r = SHCreateStreamOnFileW( pszFileName, STGM_READWRITE | STGM_CREATE | STGM_SHARE_EXCLUSIVE, &stm );
     if( SUCCEEDED( r ) )
@@ -379,9 +382,12 @@ static HRESULT WINAPI IPersistFile_fnSave(IPersistFile* iface, LPCOLESTR pszFile
    {
             StartLinkProcessor( pszFileName );

-            /* update file path */
-            heap_free(This->filepath);
-            This->filepath = strdupW(pszFileName);
+            if (fRemember)
+            {
+                /* update file path */
+                heap_free(This->filepath);
+                This->filepath = strdupW(pszFileName);
+            }

This->bDirty = FALSE;
         }
diff --git a/dlls/shell32/tests/shelllink.c b/dlls/shell32/tests/shelllink.c
index 20c48814f1..21e9107fa6 100644
--- a/dlls/shell32/tests/shelllink.c
+++ b/dlls/shell32/tests/shelllink.c
@@ -467,6 +467,16 @@ void create_lnk_(int line, const WCHAR* path, lnk_desc_t* desc)
         lok(!wcscmp(path, str), "Expected %s, got %s\n", wine_dbgstr_w(path), wine_dbgstr_w(str));
         CoTaskMemFree(str);

+        r = IPersistFile_Save(pf, NULL, TRUE);
+        lok(r == S_OK, "save failed (0x%08x)\n", r);
+
+        /* test GetCurFile after ::Save */
+        r = IPersistFile_GetCurFile(pf, &str);
+        lok(r == S_OK, "got 0x%08x\n", r);
+        lok(str != NULL, "Didn't expect NULL\n");
+        lok(!wcscmp(path, str), "Expected %s, got %s\n", wine_dbgstr_w(path), wine_dbgstr_w(str));
+        CoTaskMemFree(str);
+
         IPersistFile_Release(pf);
     }

[Dmitry Timoshkov]

Dmitry Timoshkov  wrote:
> This is a resend, previous version didn't get much attention.

Is it possible to get a review of this patch, please?

[Huw Davies]

On Wed, Feb 03, 2021 at 12:47:12PM +0300, Dmitry Timoshkov wrote:
> Dmitry Timoshkov  wrote:
> > This is a resend, previous version didn't get much attention.
>
> Is it possible to get a review of this patch, please?

What happens if you call IPersistFile_Save() with a NULL file before
a "remembered" save?

Huw.

[Dmitry Timoshkov]

Hi Huw,

Huw Davies  wrote:
> On Wed, Feb 03, 2021 at 12:47:12PM +0300, Dmitry Timoshkov wrote:
> > Dmitry Timoshkov  wrote:
> > > This is a resend, previous version didn't get much attention.
> >
> > Is it possible to get a review of this patch, please?
>
> What happens if you call IPersistFile_Save() with a NULL file before
> a "remembered" save?

Thanks for the review. In that case ::Save() returns S_OK. I'll send another
version of the patch with that fixed.

Thanks.

[Huw Davies]

On Wed, Feb 03, 2021 at 02:20:46PM +0300, Dmitry Timoshkov wrote:
> Hi Huw,
>
> Huw Davies  wrote:
> > On Wed, Feb 03, 2021 at 12:47:12PM +0300, Dmitry Timoshkov wrote:
> > > Dmitry Timoshkov  wrote:
> > > > This is a resend, previous version didn't get much attention.
> > >
> > > Is it possible to get a review of this patch, please?
> >
> > What happens if you call IPersistFile_Save() with a NULL file before
> > a "remembered" save?
>
> Thanks for the review. In that case ::Save() returns S_OK. I'll send another
> version of the patch with that fixed.

That's an interesting choice they made there.  I've sent in a v4
with some added IsDirty checks.

Huw.