Hey folks,

I have a fix here for bug 18606. I think it's decent, but I could use another few sets of eyes to make sure I didn't miss anything, or even that I'm not way on the wrong track.

The problem and solution are described in patch 3. In brief, SHChangeNotify notifies windows when a change to the filesystem occurs that the window asked to be notified about. For example, a window might tell SHChangeNotifyRegister "Inform me about changes to C:\".

Bug 18606 occurs because the new directory is passed to SHChangeNotify as '/home/user/.wine/drive_c/New Directory', which doesn't match the IDList created for 'C:\'. This patchset fixes that by converting all paths in SHChangeNotify to UNIX paths, resolving symlinks etc, and comparing the ID lists created from those paths. That way, the paths being compared are standardized and compared fairly.

Unfortunately, this is hard to write tests for. You'd have to make an ID list that is significantly different from the one SHChangeNotifyRegister will create, but that actually refers to the same directory. This is easy in Wine, but less obvious in Windows. Suggestions for how to accomplish this are welcome. Logically, though, I think the problem and solution make sense.

Please take a look.  Any feedback is appreciated.

Andrew
>From 6fdfc61f6a95acc031424b83a8c9e4ea1bb2f32e Mon Sep 17 00:00:00 2001
From: Andrew Eikum <[email protected]>
Date: Mon, 14 Jun 2010 11:03:15 -0500
Subject: shell32: Use more accurate PathIsURL instead of searching for a colon
To: wine-patches <[email protected]>
Reply-To: wine-devel <[email protected]>

This ensures that UNIX paths with a colon aren't parsed as URLs.
---
 dlls/shell32/shfldr_desktop.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/dlls/shell32/shfldr_desktop.c b/dlls/shell32/shfldr_desktop.c
index 30aea3b..54d13dc 100644
--- a/dlls/shell32/shfldr_desktop.c
+++ b/dlls/shell32/shfldr_desktop.c
@@ -193,7 +193,7 @@ static HRESULT WINAPI ISF_Desktop_fnParseDisplayName (IShellFolder2 * iface,
         *ppidl = pidlTemp;
         return S_OK;
     }
-    else if (strchrW(lpszDisplayName,':'))
+    else if (PathIsURLW(lpszDisplayName))
     {
         PARSEDURLW urldata;
 
-- 
1.7.1

>From 7155e80eff8f0c02e6cffcf09ac1056576d25507 Mon Sep 17 00:00:00 2001
From: Andrew Eikum <[email protected]>
Date: Mon, 14 Jun 2010 11:04:36 -0500
Subject: shell32: UNIX paths should be parsed by unixfs
To: wine-patches <[email protected]>
Reply-To: wine-devel <[email protected]>

---
 dlls/shell32/shfldr_desktop.c |   37 ++++++++++++++++++++++++++-----------
 1 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/dlls/shell32/shfldr_desktop.c b/dlls/shell32/shfldr_desktop.c
index 54d13dc..231c8b1 100644
--- a/dlls/shell32/shfldr_desktop.c
+++ b/dlls/shell32/shfldr_desktop.c
@@ -215,21 +215,36 @@ static HRESULT WINAPI ISF_Desktop_fnParseDisplayName (IShellFolder2 * iface,
 
         if (*lpszDisplayName)
         {
-            WCHAR szPath[MAX_PATH];
-            LPWSTR pathPtr;
-
-            /* build a complete path to create a simple pidl */
-            lstrcpynW(szPath, This->sPathTarget, MAX_PATH);
-            pathPtr = PathAddBackslashW(szPath);
-            if (pathPtr)
+            if (*lpszDisplayName == '/')
             {
-                lstrcpynW(pathPtr, lpszDisplayName, MAX_PATH - (pathPtr - szPath));
-                hr = _ILCreateFromPathW(szPath, &pidlTemp);
+                /* UNIX paths should be parsed by unixfs */
+                IShellFolder *unixFS;
+                hr = UnixFolder_Constructor(NULL, &IID_IShellFolder, (LPVOID*)&unixFS);
+                if(SUCCEEDED(hr)){
+                    hr = IShellFolder_ParseDisplayName(unixFS, NULL, NULL,
+                            lpszDisplayName, NULL, &pidlTemp, NULL);
+                    IShellFolder_Release(unixFS);
+                }
             }
             else
             {
-                /* should never reach here, but for completeness */
-                hr = HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER);
+                /* build a complete path to create a simple pidl */
+                WCHAR szPath[MAX_PATH];
+                LPWSTR pathPtr;
+
+                lstrcpynW(szPath, This->sPathTarget, MAX_PATH);
+                pathPtr = PathAddBackslashW(szPath);
+                if (pathPtr)
+                {
+                    lstrcpynW(pathPtr, lpszDisplayName, MAX_PATH - (pathPtr - szPath));
+                    TRACE("qwert: %s\n", wine_dbgstr_w(szPath));
+                    hr = _ILCreateFromPathW(szPath, &pidlTemp);
+                }
+                else
+                {
+                    /* should never reach here, but for completeness */
+                    hr = HRESULT_FROM_WIN32(ERROR_INSUFFICIENT_BUFFER);
+                }
             }
         }
         else
-- 
1.7.1

>From 7975e8157bb2056d4c60e7b83c3c3dce86be90cf Mon Sep 17 00:00:00 2001
From: Andrew Eikum <[email protected]>
Date: Fri, 11 Jun 2010 14:23:10 -0500
Subject: shell32: Convert all ID Lists to a 'standard' format in SHChangeNotify
To: wine-patches <[email protected]>
Reply-To: wine-devel <[email protected]>

This fixes bug 18606. These might be best deferred past 1.2, but it'd be
nice to fix this ugly little bug for 1.2.

SHChangeNotify sends notifications to windows when a filesystem change
matches some filter given to SHChangeNotifyRegister.  Since there are
several ways to refer to the same path with ID lists, we should make
sure to do an apples-to-apples comparison when comparing the changed path
to the paths being monitored on behalf of registered windows.

To accomplish this, we convert all paths coming into SHChangeNotify into
UNIX paths and then convert those UNIX paths to ID lists for later
comparison.  The filter given to SHChangeNotifyRegister is also
converted to the 'standard' UNIX path before comparison.

A simple example:
A window asks to be notified of changes to 'C:\'.  The user makes a
change via the shell32 API to '/home/user/.wine/drive_c'.

Previously, this change would not have been sent to the window because
'/home/user/.wine/drive_c' is not obviously the same as 'C:\'.

By converting to UNIX paths, however, the 'C:\' filter becomes
'/home/user/.wine/drive_c' and the window becomes notified of the change.
---
 dlls/shell32/changenotify.c |  112 +++++++++++++++++++++++++++++++++++++------
 dlls/shell32/pidl.c         |    2 +-
 dlls/shell32/undocshell.h   |    5 ++
 3 files changed, 103 insertions(+), 16 deletions(-)

diff --git a/dlls/shell32/changenotify.c b/dlls/shell32/changenotify.c
index 95f2e42..2ec4af7 100644
--- a/dlls/shell32/changenotify.c
+++ b/dlls/shell32/changenotify.c
@@ -18,6 +18,7 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  */
 
+#include <limits.h>
 #include <stdarg.h>
 #include <string.h>
 
@@ -28,6 +29,7 @@
 #include "wine/list.h"
 #include "wine/debug.h"
 #include "shell32_main.h"
+#include "undocshell.h"
 
 WINE_DEFAULT_DEBUG_CHANNEL(shell);
 
@@ -248,17 +250,81 @@ static BOOL should_notify( LPCITEMIDLIST changed, LPCITEMIDLIST watched, BOOL su
     return FALSE;
 }
 
+/* returns a PIDL from the path in a standardized format such that paths
+ * like 'C:\' and '/home/user/.wine/drive_c' make ILIsEqual return TRUE
+ *
+ * the return value of this function should be SHFreed */
+static LPITEMIDLIST standardized_pidl_from_pathW(LPCWSTR path)
+{
+    char *unix_path, real_path[PATH_MAX];
+    LPITEMIDLIST ret;
+    HRESULT hr = 0;
+
+    if (!path)
+        return NULL;
+
+    if ((unix_path = wine_get_unix_file_name(path)) == NULL ||
+            realpath(unix_path, real_path) == NULL ||
+            strlen(real_path) > MAX_PATH ||
+            FAILED((hr = SHILCreateFromPathA(real_path, &ret, NULL))))
+    {
+        WARN("Unable to standardize path (%s): 0x%08x\n", wine_dbgstr_w(path), hr);
+        HeapFree(GetProcessHeap(), 0, unix_path);
+        return NULL;
+    }
+
+    HeapFree(GetProcessHeap(), 0, unix_path);
+
+    return ret;
+}
+
+static LPITEMIDLIST standardized_pidl_from_pathA(LPCSTR path)
+{
+    WCHAR pathW[MAX_PATH];
+
+    if(!path ||
+            !MultiByteToWideChar(CP_ACP, 0, path, -1, pathW, MAX_PATH)){
+        WARN("Unable to convert to WCHAR: %s\n", path);
+        return NULL;
+    }
+
+    return standardized_pidl_from_pathW(pathW);
+}
+
+/* see standardized_pidl_from_path
+ *
+ * if an error occurs, a copy of `pidl' is returned */
+static LPITEMIDLIST standardize_pidl(LPCITEMIDLIST pidl)
+{
+    WCHAR path[MAX_PATH];
+    LPITEMIDLIST ret;
+
+    if (!pidl)
+        return NULL;
+
+    if (!SHGetPathFromIDListW(pidl, path) ||
+            (ret = standardized_pidl_from_pathW(path)) == NULL)
+    {
+        WARN("Unable to standardize ID list %p\n", pidl);
+        return ILClone(pidl);
+    }
+
+    return ret;
+}
+
 /*************************************************************************
  * SHChangeNotify				[shell...@]
  */
 void WINAPI SHChangeNotify(LONG wEventId, UINT uFlags, LPCVOID dwItem1, LPCVOID dwItem2)
 {
-    LPCITEMIDLIST Pidls[2];
+    LPCITEMIDLIST Pidls[2], std_pidls[2];
     LPNOTIFICATIONLIST ptr;
     UINT typeFlag = uFlags & SHCNF_TYPE;
 
     Pidls[0] = NULL;
     Pidls[1] = NULL;
+    std_pidls[0] = NULL;
+    std_pidls[1] = NULL;
 
     TRACE("(0x%08x,0x%08x,%p,%p):stub.\n", wEventId, uFlags, dwItem1, dwItem2);
 
@@ -291,16 +357,34 @@ void WINAPI SHChangeNotify(LONG wEventId, UINT uFlags, LPCVOID dwItem1, LPCVOID
     switch (typeFlag)
     {
     case SHCNF_PATHA:
-        if (dwItem1) Pidls[0] = SHSimpleIDListFromPathA(dwItem1);
-        if (dwItem2) Pidls[1] = SHSimpleIDListFromPathA(dwItem2);
+        if (dwItem1)
+        {
+            Pidls[0] = SHSimpleIDListFromPathA(dwItem1);
+            std_pidls[0] = standardized_pidl_from_pathA(dwItem1);
+        }
+        if (dwItem2)
+        {
+            Pidls[1] = SHSimpleIDListFromPathA(dwItem2);
+            std_pidls[1] = standardized_pidl_from_pathA(dwItem2);
+        }
         break;
     case SHCNF_PATHW:
-        if (dwItem1) Pidls[0] = SHSimpleIDListFromPathW(dwItem1);
-        if (dwItem2) Pidls[1] = SHSimpleIDListFromPathW(dwItem2);
+        if (dwItem1)
+        {
+            Pidls[0] = SHSimpleIDListFromPathW(dwItem1);
+            std_pidls[0] = standardized_pidl_from_pathW(dwItem1);
+        }
+        if (dwItem2)
+        {
+            Pidls[1] = SHSimpleIDListFromPathW(dwItem2);
+            std_pidls[1] = standardized_pidl_from_pathW(dwItem2);
+        }
         break;
     case SHCNF_IDLIST:
         Pidls[0] = dwItem1;
+        std_pidls[0] = standardize_pidl(dwItem1);
         Pidls[1] = dwItem2;
+        std_pidls[1] = standardize_pidl(dwItem2);
         break;
     case SHCNF_PRINTERA:
     case SHCNF_PRINTERW:
@@ -312,7 +396,7 @@ void WINAPI SHChangeNotify(LONG wEventId, UINT uFlags, LPCVOID dwItem1, LPCVOID
         return;
     }
 
-    {
+    if (TRACE_ON(shell)){
         WCHAR path[MAX_PATH];
 
         if( Pidls[0] && SHGetPathFromIDListW(Pidls[0], path ))
@@ -336,7 +420,7 @@ void WINAPI SHChangeNotify(LONG wEventId, UINT uFlags, LPCVOID dwItem1, LPCVOID
 
         for( i=0; (i<ptr->cidl) && !notify ; i++ )
         {
-            LPCITEMIDLIST pidl = ptr->apidl[i].pidl;
+            LPCITEMIDLIST pidl = standardize_pidl(ptr->apidl[i].pidl);
             BOOL subtree = ptr->apidl[i].fRecursive;
 
             if (wEventId & ptr->wEventMask)
@@ -346,10 +430,12 @@ void WINAPI SHChangeNotify(LONG wEventId, UINT uFlags, LPCVOID dwItem1, LPCVOID
                 else if( wEventId & SHCNE_NOITEMEVENTS )
                     notify = TRUE;
                 else if( wEventId & ( SHCNE_ONEITEMEVENTS | SHCNE_TWOITEMEVENTS ) )
-                    notify = should_notify( Pidls[0], pidl, subtree );
+                    notify = should_notify( std_pidls[0], pidl, subtree );
                 else if( wEventId & SHCNE_TWOITEMEVENTS )
-                    notify = should_notify( Pidls[1], pidl, subtree );
+                    notify = should_notify( std_pidls[1], pidl, subtree );
             }
+
+            SHFree((LPITEMIDLIST)pidl);
         }
 
         if( !notify )
@@ -381,12 +467,8 @@ void WINAPI SHChangeNotify(LONG wEventId, UINT uFlags, LPCVOID dwItem1, LPCVOID
         run_winemenubuilder( args );
     }
 
-    /* if we allocated it, free it. The ANSI flag is also set in its Unicode sibling. */
-    if ((typeFlag & SHCNF_PATHA) || (typeFlag & SHCNF_PRINTERA))
-    {
-        SHFree((LPITEMIDLIST)Pidls[0]);
-        SHFree((LPITEMIDLIST)Pidls[1]);
-    }
+    SHFree((LPITEMIDLIST)std_pidls[0]);
+    SHFree((LPITEMIDLIST)std_pidls[1]);
 }
 
 /*************************************************************************
diff --git a/dlls/shell32/pidl.c b/dlls/shell32/pidl.c
index dffa2bd..1192c10 100644
--- a/dlls/shell32/pidl.c
+++ b/dlls/shell32/pidl.c
@@ -381,7 +381,7 @@ HRESULT WINAPI ILSaveToStream (IStream * pStream, LPCITEMIDLIST pPidl)
  * NOTES
  *  Wrapper for IShellFolder_ParseDisplayName().
  */
-static HRESULT SHILCreateFromPathA(LPCSTR path, LPITEMIDLIST * ppidl, DWORD * attributes)
+HRESULT SHILCreateFromPathA(LPCSTR path, LPITEMIDLIST * ppidl, DWORD * attributes)
 {
     WCHAR lpszDisplayName[MAX_PATH];
 
diff --git a/dlls/shell32/undocshell.h b/dlls/shell32/undocshell.h
index 513006d..772f082 100644
--- a/dlls/shell32/undocshell.h
+++ b/dlls/shell32/undocshell.h
@@ -55,6 +55,11 @@ void WINAPI ILGlobalFree(LPITEMIDLIST pidl);
 LPITEMIDLIST SHSimpleIDListFromPathA (LPCSTR lpszPath);
 LPITEMIDLIST SHSimpleIDListFromPathW (LPCWSTR lpszPath);
 
+HRESULT SHILCreateFromPathA (
+	LPCSTR path,
+	LPITEMIDLIST * ppidl,
+	DWORD *attributes);
+
 HRESULT SHILCreateFromPathW (
 	LPCWSTR path,
 	LPITEMIDLIST * ppidl,
-- 
1.7.1



Reply via email to