Re: CommandLineToArgvW fix

[eric pouech]

Malte Starostik wrote:
> Don't free cmdline as the return values point into that string. It's the
> caller's responsibility to GlobalFree() this according to msdn.

I don't understand what you've done.
cmdline is a copy of the passed lpCmdLine parameter used for getting the
different bits of the argv, so it should be freed
however, what's wrong with current implementation (hence your issues) is
that when freeing cmdline the argv array points to garbage
it means that the argv array and cmdline should be allocated in a single 
memory block suh that argv[i] points to the part of the string

wouldn't the (completly untested but) attached patch work better ?

A+
-- 
---------------
Eric Pouech (http://perso.wanadoo.fr/eric.pouech/)
"The future will be better tomorrow", Vice President Dan Quayle

Index: dlls/shell32/shell32_main.c
===================================================================
RCS file: /usr/share/cvs/cvsroot/wine/wine/dlls/shell32/shell32_main.c,v
retrieving revision 1.85
diff -u -r1.85 shell32_main.c
--- dlls/shell32/shell32_main.c	2001/10/03 18:42:16	1.85
+++ dlls/shell32/shell32_main.c	2001/10/08 18:06:34
@@ -1,3 +1,4 @@
+
 /*
  * 				Shell basics
  *
@@ -88,15 +89,10 @@
         return argv;
     }
 
-    /* to get a writeable copy */
-    cmdline = HeapAlloc(GetProcessHeap(), 0, (strlenW(lpCmdline)+1) * sizeof(WCHAR));
-    if (!cmdline)
-        return NULL;
-    strcpyW(cmdline, lpCmdline);
     argc=0;
     bcount=0;
     in_quotes=0;
-    s=cmdline;
+    s=(LPWSTR)lpCmdline;
     while (1) {
         if (*s==0 || ((*s==0x0009 || *s==0x0020) && !in_quotes)) {
             /* space */
@@ -122,7 +118,12 @@
         }
         s++;
     }
-    argv=HeapAlloc(GetProcessHeap(), 0, (argc+1)*sizeof(LPWSTR));
+    argv=HeapAlloc(GetProcessHeap(), 0, (argc+1)*sizeof(LPWSTR) + strlenW(lpCmdline) + 1);
+    if (!argv)
+        return NULL;
+    /* to get a writeable copy */
+    cmdline = (LPWSTR)((char*)argv + (argc+1)*sizeof(LPWSTR));
+    strcpyW(cmdline, lpCmdline);
 
     argc=0;
     bcount=0;
@@ -178,7 +179,6 @@
     if (numargs)
         *numargs=argc;
 
-    HeapFree(GetProcessHeap(), 0, cmdline);
     return argv;
 }

CommandLineToArgvW fix

[eric pouech]

> wouldn't the (completly untested but) attached patch work better ?
> +    argv=HeapAlloc(GetProcessHeap(), 0, (argc+1)*sizeof(LPWSTR) + strlenW(lpCmdline) + 1);
> hmm, it should of course be
> +    argv=HeapAlloc(GetProcessHeap(), 0, (argc+1)*sizeof(LPWSTR) + (strlenW(lpCmdline) + 1) * sizeof(WCHAR));

A+
-- 
---------------
Eric Pouech (http://perso.wanadoo.fr/eric.pouech/)
"The future will be better tomorrow", Vice President Dan Quayle