[dlls/shell32/shell.c] Strncpy elimnation :-) Rewrite of DoEnvironmentSubst16

[Peter Berg Larsen]

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/shellcc/platform/shell/reference/functions/doenvironmentsubst.asp

The old code did not leave str intact if the keyword was not found, as it
returned false before rewriting the '%'.  Secondly it seems that it is
perfectly valid to miss a keyword, the %keyword% should be left intact. Do
test this rewrite some more.

Changelog:
         Rewritten DoEnvironmentSubst16.


Index: dlls/shell32/shell.c
===================================================================
RCS file: /home/wine/wine/dlls/shell32/shell.c,v
retrieving revision 1.62
diff -u -r1.62 shell.c
--- dlls/shell32/shell.c        21 Mar 2005 11:25:13 -0000      1.62
+++ dlls/shell32/shell.c        15 Apr 2005 20:14:43 -0000
@@ -363,74 +363,82 @@
  *                             DoEnvironmentSubst      [SHELL.37]
  *
  * Replace %KEYWORD% in the str with the value of variable KEYWORD
- * from "DOS" environment.
+ * from "DOS" environment. If it is not found the %KEYWORD% is left
+ * intact. If the buffer is too small, str is not modified.
+ *
+ * str         [I] '\0' terminated string with %keyword%.
+ *             [O] '\0' terminated string with %keyword% substituted.
+ * length      [I] size of str.
+ *
+ * Return
+ *     str length in the LOWORD and 1 in HIWORD if subst was successful.
  */
 DWORD WINAPI DoEnvironmentSubst16(LPSTR str,WORD length)
 {
   LPSTR   lpEnv = MapSL(GetDOSEnvironment16());
-  LPSTR   lpBuffer = HeapAlloc( GetProcessHeap(), 0, length);
   LPSTR   lpstr = str;
-  LPSTR   lpbstr = lpBuffer;
+  LPSTR   lpend;
+  LPSTR   lpBuffer = HeapAlloc( GetProcessHeap(), 0, length);
+  WORD    bufCnt = 0;
+  WORD    envKeyLen;
+  LPSTR   lpKey;
+  WORD    retStatus = 0;
+  WORD    retLength = length;

   CharToOemA(str,str);

   TRACE("accept %s\n", str);

-  while( *lpstr && lpbstr - lpBuffer < length )
-   {
-     LPSTR lpend = lpstr;
-
-     if( *lpstr == '%' )
-       {
-         do { lpend++; } while( *lpend && *lpend != '%' );
-         if( *lpend == '%' && lpend - lpstr > 1 )      /* found key */
-           {
-              LPSTR lpKey;
-             *lpend = '\0';
-              lpKey = SHELL_FindString(lpEnv, lpstr+1);
-              if( lpKey )                              /* found key value */
-                {
-                  int l = strlen(lpKey);
-
-                  if( l > length - (lpbstr - lpBuffer) - 1 )
-                    {
-           WARN("-- Env subst aborted - string too short\n");
-                     *lpend = '%';
-                      break;
-                    }
-                  strcpy(lpbstr, lpKey);
-                  lpbstr += l;
-                }
-              else break;
-             *lpend = '%';
-              lpstr = lpend + 1;
-           }
-         else break;                                   /* back off and whine */
-
-         continue;
-       }
-
-     *lpbstr++ = *lpstr++;
-   }
-
- *lpbstr = '\0';
-  if( lpstr - str == strlen(str) )
-    {
-      strncpy(str, lpBuffer, length);
-      length = 1;
-    }
-  else
-      length = 0;
-
+  while( *lpstr && bufCnt <= length - 1 ) {
+     if ( *lpstr != '%' ) {
+        lpBuffer[bufCnt++] = *lpstr++;
+        continue;
+     }
+
+     for( lpend = lpstr + 1; *lpend && *lpend != '%'; lpend++) /**/;
+
+     envKeyLen = lpend - lpstr - 1;
+     if( *lpend != '%' || envKeyLen == 0)
+        goto err; /* "%\0" or "%%" found; back off and whine */
+
+     *lpend = '\0';
+     lpKey = SHELL_FindString(lpEnv, lpstr+1);
+     *lpend = '%';
+     if( lpKey ) {
+         int l = strlen(lpKey);
+
+         if( bufCnt + l > length - 1 )
+                goto err;
+
+        memcpy(lpBuffer + bufCnt, lpKey, l);
+        bufCnt += l;
+     } else { /* Keyword not found; Leave the %KEYWORD% intact */
+        if( bufCnt + envKeyLen + 2 > length - 1 )
+            goto err;
+
+         memcpy(lpBuffer + bufCnt, lpstr, envKeyLen + 2);
+        bufCnt += envKeyLen + 2;
+     }
+
+     lpstr = lpend + 1;
+  }
+
+  if (!*lpstr && bufCnt <= length - 1) {
+      memcpy(str,lpBuffer, bufCnt);
+      str[bufCnt] = '\0';
+      retLength = bufCnt + 1;
+      retStatus = 1;
+  }
+
+  err:
+  if (!retStatus)
+      WARN("-- Env subst aborted - string too short or invalid input\n");
   TRACE("-- return %s\n", str);

   OemToCharA(str,str);
   HeapFree( GetProcessHeap(), 0, lpBuffer);

-  /*  Return str length in the LOWORD
-   *  and 1 in HIWORD if subst was successful.
-   */
- return (DWORD)MAKELONG(strlen(str), length);
+  return (DWORD)MAKELONG(retLength, retStatus);
 }


/*************************************************************************