Fwd: Re: CommandLineToArgvW fix

[Francois Gouget]

On Mon, 8 Oct 2001, Malte Starostik wrote:

> Sorry, meant reply-to-all
> 
> ----------  Weitergeleitete Nachricht  ----------
> 
> Subject: Re: CommandLineToArgvW fix
> Date: Mon, 8 Oct 2001 20:23:57 +0200
> From: Malte Starostik <malte at kde.org>
> To: eric pouech <eric.pouech at wanadoo.fr>
> 
> Am Monday 08 October 2001 20:08 schrieben Sie:
> > 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
> 
> You're definately right, I mixed up argv with argv[i] there.


   Here's my take on this. I believe it's a slightly better patch.
   I tested this in NT4 and there is indeed a single alloc done for the
array and the strings. Also, the array does not contain a terminating
NULL pointer like I thought. And the lpCmdLine="" case is handled the
same way so this patch changes that too.
   I did not test the patch... yet (recompiling). If I find something
wrong I'll send an updated version.


--
Francois Gouget         fgouget at free.fr        http://fgouget.free.fr/
  Any sufficiently advanced Operating System is indistinguishable from Linux
-------------- next part --------------
Index: dlls/shell32/shell32_main.c
===================================================================
RCS file: /home/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 17:12:35
@@ -65,6 +65,7 @@
 {
     DWORD argc;
     LPWSTR  *argv;
+    LPCWSTR cs;
     LPWSTR arg,s,d;
     LPWSTR cmdline;
     int in_quotes,bcount;
@@ -74,14 +75,13 @@
         /* Return the path to the executable */
         DWORD size;
 
-        argv=HeapAlloc(GetProcessHeap(), 0, 2*sizeof(LPWSTR));
-        argv[0]=NULL;
+        argv=NULL;
         size=16;
         do {
             size*=2;
-            argv[0]=HeapReAlloc(GetProcessHeap(), 0, argv[0], size);
-        } while (GetModuleFileNameW((HMODULE)0, argv[0], size) == 0);
-        argv[1]=NULL;
+            argv=HeapReAlloc(GetProcessHeap(), 0, argv, size);
+        } while (GetModuleFileNameW((HMODULE)0, (LPWSTR)(argv+1), size-sizeof(LPWSTR)) == 0);
+        argv[0]=(LPWSTR)(argv+1);
         if (numargs)
             *numargs=2;
 
@@ -89,30 +89,26 @@
     }
 
     /* 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;
+    cs=lpCmdline;
     while (1) {
-        if (*s==0 || ((*s==0x0009 || *s==0x0020) && !in_quotes)) {
+        if (*cs==0 || ((*cs==0x0009 || *cs==0x0020) && !in_quotes)) {
             /* space */
             argc++;
             /* skip the remaining spaces */
-            while (*s==0x0009 || *s==0x0020) {
-                s++;
+            while (*cs==0x0009 || *cs==0x0020) {
+                cs++;
             }
-            if (*s==0)
+            if (*cs==0)
                 break;
             bcount=0;
             continue;
-        } else if (*s==0x005c) {
+        } else if (*cs==0x005c) {
             /* '\', count them */
             bcount++;
-        } else if ((*s==0x0022) && ((bcount & 1)==0)) {
+        } else if ((*cs==0x0022) && ((bcount & 1)==0)) {
             /* unescaped '"' */
             in_quotes=!in_quotes;
             bcount=0;
@@ -120,9 +116,16 @@
             /* a regular character */
             bcount=0;
         }
-        s++;
+        cs++;
     }
-    argv=HeapAlloc(GetProcessHeap(), 0, (argc+1)*sizeof(LPWSTR));
+    /* Allocate in a single lump, the string array, and the strings that go with it.
+     * This way the caller can make a single GlobalFree call to free both, as per MSDN.
+     */
+    argv=HeapAlloc(GetProcessHeap(), 0, argc*sizeof(LPWSTR)+(strlenW(lpCmdline)+1)*sizeof(WCHAR));
+    if (!argv)
+        return NULL;
+    cmdline=(LPWSTR)(argv+argc);
+    strcpyW(cmdline, lpCmdline);
 
     argc=0;
     bcount=0;
@@ -172,13 +175,11 @@
     }
     if (*arg) {
         *d='\0';
-        argv[argc++]=arg;
+        argv[argc]=arg;
     }
-    argv[argc]=NULL;
     if (numargs)
         *numargs=argc;
 
-    HeapFree(GetProcessHeap(), 0, cmdline);
     return argv;
 }