RE: [SHLWAPI] Implemented PathSearchAndQualify (fix)

[Rolf Kalbermatter]

A few comments:

/*************************************************************************
@@ -3126,8 +3146,87 @@
  */
 BOOL WINAPI PathSearchAndQualifyW(LPCWSTR lpszPath, LPWSTR lpszBuf, UINT cchBuf)
 {
-  FIXME("(%s,%p,0x%08x)-stub\n", debugstr_w(lpszPath), lpszBuf, cchBuf);
-  return FALSE;
+  LPWSTR pathCopy;
+  LPWSTR bufPos = lpszBuf;
+  LPWSTR bufPos2 = bufPos;
+  BOOL ret = TRUE;
+  
+  if ( !(pathCopy = HeapAlloc(GetProcessHeap(), 0, cchBuf * sizeof(WCHAR))))
+  	return FALSE;
+  StrCpyW(pathCopy, lpszPath);
+  lpszPath = pathCopy;

This is rather ugly. You shouldn't overwrite the pointer to the passed in
buffer. Use for internal pointer reference handling one of the local variables
instead of overwriting a parameter variable.

+  TRACE("(%s %p %d)\n", debugstr_w(lpszPath), lpszBuf, cchBuf);
+
+  if (!lpszPath || !*lpszPath)
+    return FALSE;

I think this is bad order! The trace should be on entry and definitely
before you modify any passed in parameters. The same applies for the
valid input parameter check.

+  if (*pathCopy == '\' && pathCopy[1] == '\')
+  {
+    /* Network share: skip share server and mount point */
+    pathCopy += 2;
+    if ((pathCopy = StrChrW(pathCopy, '\')) &&
+        (pathCopy = StrChrW(pathCopy + 1, '\')))
+      pathCopy++;
+  }
+
+  /* Check x:\ */
+  if (pathCopy[0] && pathCopy[1] == ':' && pathCopy[2] == '\')
+    pathCopy += 3;
+
+  if ( pathCopy != lpszPath )
+  {
+    StrCpyNW(bufPos, lpszPath, (pathCopy - lpszPath) +1);
+    bufPos += (pathCopy - lpszPath);
+    bufPos2 = bufPos;
+    *bufPos = '\0';
+    lpszPath = pathCopy;
+  } else {
+    FIXME("must prepend current path");
+  }
+
+  while(*pathCopy)
+  {
+    if ( *pathCopy == '.' )
+    {
+      if ( ! pathCopy[1] )
+      {
+        ret = TRUE;
+        break;
+      }
+
+      if ( pathCopy[1] == '\' ) { pathCopy += 2; continue; }
+        
+      if ( pathCopy[1] == '.' && pathCopy[2] == '\' )
+      {
+        if ( bufPos == bufPos2 )
+        {
+          pathCopy += 3;
+          continue;
+        }
+
+        bufPos = StrRChrW(bufPos, bufPos2, '\');
+        *bufPos = '\0';
+        continue;
+      }
+    }
+
+    if ( StrChrW(pathCopy, '\') )
+    {
+      size_t len = (StrChrW(pathCopy, '\') - pathCopy )+1;
+      StrCpyNW(bufPos, pathCopy, len+1);
+      bufPos += len;
+      pathCopy += len;
+      *bufPos = '\0';
+      continue;
+    } else {
+      StrCpyW(bufPos, pathCopy);
+      break;
+    }
+  }
+
+  HeapFree(GetProcessHeap(), 0, pathCopy);
+  return ret;
 }

I don't see how pathCopy can be the original pointer here after having made
so much of pointer increments on that variable inside the function. This
would mean that HeapFree will completely fail or probably just crash.

Rolf Kalbermatter