[PATCH 2/5] cmd: use GetCommandline() rather than argv

Martin Wilck mwilck at arcor.de
Mon Aug 29 17:34:31 CDT 2011


Windows CMD.EXE doesn't use argc/argv for the command line arguments.
Automatic argv processing strips most quotes from the command line,
leading to wrong results (try cmd /c @echo "hi").

This patch makes CMD use GetCommandLine() instead. Some code that
served to work around argv parsing artefacts can be dropped.

The TODOs in the test results are also changed. Besides improvement,
this patch causes a few regressions that will be fixed in follow-up
patches. A problem in the new test code is also fixed.
---
 programs/cmd/tests/test_parsing.cmd     |    2 +
 programs/cmd/tests/test_parsing.cmd.exp |   20 ++--
 programs/cmd/wcmdmain.c                 |  249 +++++++++++--------------------
 3 files changed, 101 insertions(+), 170 deletions(-)

diff --git a/programs/cmd/tests/test_parsing.cmd b/programs/cmd/tests/test_parsing.cmd
index 4db6b94..10bb846 100644
--- a/programs/cmd/tests/test_parsing.cmd
+++ b/programs/cmd/tests/test_parsing.cmd
@@ -30,7 +30,9 @@ if errorlevel 1 echo error %ErrorLevel%
 echo ------ Testing invocation with CMD /C -------------
 cmd /c say one
 cmd /c "say one"
+call :setError 0
 cmd /c "say"" one"
+if errorlevel 1 echo error %ErrorLevel%
 cmd /c "say one
 call :setError 0
 cmd /c say" one"
diff --git a/programs/cmd/tests/test_parsing.cmd.exp b/programs/cmd/tests/test_parsing.cmd.exp
index 4d07801..fe35a30 100644
--- a/programs/cmd/tests/test_parsing.cmd.exp
+++ b/programs/cmd/tests/test_parsing.cmd.exp
@@ -15,26 +15,26 @@
 0 at space@
 1 at space@
 @todo_wine at 0@space@
- at todo_wine@0 at space@
-1 at space@
 0 at space@
-1 at space@
-2 at space@
+ at todo_wine@1 at space@
+0 at space@
+ at todo_wine@1 at space@
+ at todo_wine@2 at space@
 0 at space@
 0 at space@
 @todo_wine at 3@space@
 @todo_wine at 4@space@
 ---------- Testing CMD /C quoting -----------------
- at todo_wine@"hi"
+"hi"
+ at todo_wine@1 at space@
+"\"\\"\\\"\\\\"@space@"\"\\"\\\"\\\\"
 1 at space@
- at todo_wine@"\"\\"\\\"\\\\"@space@"\"\\"\\\"\\\\"
+0 at space@
 1 at space@
 0 at space@
- at todo_wine@1 at space@
- at todo_wine@0 at space@
 @todo_wine at 0@space@
 0 at space@
- at todo_wine@3 at space@
+3 at space@
 2 at space@
 2 at space@
 2 at space@
@@ -45,7 +45,7 @@
 1 at space@
 0 at space@
 good
- at todo_wine@good
+good
 --------- Testing special characters --------------
 0 at space@
 @todo_wine at 0@space@
diff --git a/programs/cmd/wcmdmain.c b/programs/cmd/wcmdmain.c
index b282e99..a90eb29 100644
--- a/programs/cmd/wcmdmain.c
+++ b/programs/cmd/wcmdmain.c
@@ -2221,6 +2221,68 @@ void WCMD_free_commands(CMD_LIST *cmds) {
     }
 }
 
+/*
+ * Helper for wmain - Skips program name from command line.
+ */
+static const WCHAR*  skip_program_name(const WCHAR *cmdline)
+{
+    const WCHAR *p;
+    BOOL in_quotes;
+    for (p = cmdline, in_quotes = FALSE; *p != '\0'; p++) {
+	if (!in_quotes && (*p == ' ' || *p == '\t')) {
+	    do { p++; } while (*p == ' ' || *p == '\t');
+	    break;
+	}
+	if (*p == '"')
+	    in_quotes = !in_quotes;
+    }
+    return p;
+}
+
+/*
+ * If the first /c or /k command character is a quote, cmd follows
+ * a complex logic to decide whether quotes will be kept:
+ *   1. no /s switch
+ *   2. exactly 2 quotes
+ *   3. at least 1 whitespace between the quotes
+ *   4. no special char (see below) between the quotes
+ *   5. string between quotes is a valid exe or bat file
+ * This function checks 2.-4., 5. is checked in WCMD_run_program().
+ */
+static BOOL may_keep_quotes(const WCHAR *chr)
+{
+    const WCHAR *pt, *pq;
+    BOOL spc;
+
+    for (pt = chr+1, spc = FALSE, pq = NULL; *pt != '\0'; pt++) {
+	  if (*pt == '"') {
+	      if (pq != NULL)
+		  /* don't keep if > 2 quotes */
+		  return FALSE;
+	      pq = pt;
+	      continue;
+	  }
+	  if (pq != NULL)
+	      /* already behind closing quote */
+	      continue;
+	  switch (*pt) {
+	  case ' ': case '\t':
+	      spc = TRUE;
+	      break;
+	  /* don't keep if text between quotes contains special character */
+	  case '&': case '<': case '>': case '(':
+	  case ')': case '@': case '^': case '|':
+	      return FALSE;
+	  default:
+	      break;
+	  }
+    }
+
+    /* don't keep if no space between the quotes, or if < 2 quotes */
+    if (!spc || pq == NULL)
+	return FALSE;
+    return TRUE;
+}
 
 /*****************************************************************************
  * Main entry point. This is a console application so we have a main() not a
@@ -2229,7 +2291,6 @@ void WCMD_free_commands(CMD_LIST *cmds) {
 
 int wmain (int argc, WCHAR *argvW[])
 {
-  int     args;
   WCHAR  *cmd   = NULL;
   WCHAR string[1024];
   WCHAR envvar[4];
@@ -2242,6 +2303,7 @@ int wmain (int argc, WCHAR *argvW[])
   static const WCHAR defaultpromptW[] = {'$','P','$','G','\0'};
   char ansiVersion[100];
   CMD_LIST *toExecute = NULL;         /* Commands left to be executed */
+  WCHAR *cmdline, *chr;
 
   srand(time(NULL));
 
@@ -2251,19 +2313,22 @@ int wmain (int argc, WCHAR *argvW[])
   wsprintfW(version_string, WCMD_LoadMessage(WCMD_VERSION), string);
   strcpyW(anykey, WCMD_LoadMessage(WCMD_ANYKEY));
 
-  args  = argc;
-  opt_c=opt_k=opt_q=opt_s=0;
-  while (args > 0)
-  {
-      WCHAR c;
-      WINE_TRACE("Command line parm: '%s'\n", wine_dbgstr_w(*argvW));
-      if ((*argvW)[0]!='/' || (*argvW)[1]=='\0') {
-          argvW++;
-          args--;
-          continue;
-      }
+  cmdline = WCMD_strdupW(GetCommandLineW());
+  if (cmdline == NULL) {
+      SetLastError(ERROR_OUTOFMEMORY);
+      exit(1);
+  }
+  chr = (WCHAR*)skip_program_name(cmdline);
+  WINE_TRACE("Command line: '%s'\n", wine_dbgstr_w(chr));
 
-      c=(*argvW)[1];
+  opt_c=opt_k=opt_q=opt_s=0;
+  for (; *chr != '\0'; chr++) {
+      WCHAR c, *t;
+      if (*chr != '/')
+	  continue;
+      c = *(++chr);
+      if (c == '\0')
+	  break;
       if (tolowerW(c)=='c') {
           opt_c=1;
       } else if (tolowerW(c)=='q') {
@@ -2276,21 +2341,15 @@ int wmain (int argc, WCHAR *argvW[])
           unicodePipes=FALSE;
       } else if (tolowerW(c)=='u') {
           unicodePipes=TRUE;
-      } else if (tolowerW(c)=='t' && (*argvW)[2]==':') {
-          opt_t=strtoulW(&(*argvW)[3], NULL, 16);
+      } else if (tolowerW(c)=='t' && *(chr+1) == ':') {
+          opt_t=strtoulW(chr+2, &t, 16);
+	  if (t == chr+2)
+	      opt_t = 0;
+	  chr = t-1; /* will be incremented in for loop */
       } else if (tolowerW(c)=='x' || tolowerW(c)=='y') {
           /* Ignored for compatibility with Windows */
       }
 
-      if ((*argvW)[2]==0) {
-          argvW++;
-          args--;
-      }
-      else /* handle `cmd /cnotepad.exe` and `cmd /x/c ...` */
-      {
-          *argvW+=2;
-      }
-
       if (opt_c || opt_k) /* break out of parsing immediately after c or k */
           break;
   }
@@ -2301,147 +2360,17 @@ int wmain (int argc, WCHAR *argvW[])
   }
 
   if (opt_c || opt_k) {
-      int     len,qcount;
-      WCHAR** arg;
-      int     argsLeft;
-      WCHAR*  p;
-
-      /* opt_s left unflagged if the command starts with and contains exactly
-       * one quoted string (exactly two quote characters). The quoted string
-       * must be an executable name that has whitespace and must not have the
-       * following characters: &<>()@^| */
-
-      /* Build the command to execute */
-      len = 0;
-      qcount = 0;
-      argsLeft = args;
-      for (arg = argvW; argsLeft>0; arg++,argsLeft--)
-      {
-          int has_space,bcount;
-          WCHAR* a;
-
-          has_space=0;
-          bcount=0;
-          a=*arg;
-          if( !*a ) has_space=1;
-          while (*a!='\0') {
-              if (*a=='\\') {
-                  bcount++;
-              } else {
-                  if (*a==' ' || *a=='\t') {
-                      has_space=1;
-                  } else if (*a=='"') {
-                      /* doubling of '\' preceding a '"',
-                       * plus escaping of said '"'
-                       */
-                      len+=2*bcount+1;
-                      qcount++;
-                  }
-                  bcount=0;
-              }
-              a++;
-          }
-          len+=(a-*arg) + 1; /* for the separating space */
-          if (has_space)
-          {
-              len+=2; /* for the quotes */
-              qcount+=2;
-          }
-      }
-
-      if (qcount!=2)
-          opt_s=1;
-
-      /* check argvW[0] for a space and invalid characters */
-      if (!opt_s) {
-          opt_s=1;
-          p=*argvW;
-          while (*p!='\0') {
-              if (*p=='&' || *p=='<' || *p=='>' || *p=='(' || *p==')'
-                  || *p=='@' || *p=='^' || *p=='|') {
-                  opt_s=1;
-                  break;
-              }
-              if (*p==' ')
-                  opt_s=0;
-              p++;
-          }
-      }
-
-      cmd = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
-      if (!cmd)
-          exit(1);
 
-      p = cmd;
-      argsLeft = args;
-      for (arg = argvW; argsLeft>0; arg++,argsLeft--)
-      {
-          int has_space,has_quote;
-          WCHAR* a;
-
-          /* Check for quotes and spaces in this argument */
-          has_space=has_quote=0;
-          a=*arg;
-          if( !*a ) has_space=1;
-          while (*a!='\0') {
-              if (*a==' ' || *a=='\t') {
-                  has_space=1;
-                  if (has_quote)
-                      break;
-              } else if (*a=='"') {
-                  has_quote=1;
-                  if (has_space)
-                      break;
-              }
-              a++;
-          }
+      for (cmd = chr+1; *cmd == ' ' || *cmd == '\t'; cmd++);
 
-          /* Now transfer it to the command line */
-          if (has_space)
-              *p++='"';
-          if (has_quote) {
-              int bcount;
-              WCHAR* a;
-
-              bcount=0;
-              a=*arg;
-              while (*a!='\0') {
-                  if (*a=='\\') {
-                      *p++=*a;
-                      bcount++;
-                  } else {
-                      if (*a=='"') {
-                          int i;
-
-                          /* Double all the '\\' preceding this '"', plus one */
-                          for (i=0;i<=bcount;i++)
-                              *p++='\\';
-                          *p++='"';
-                      } else {
-                          *p++=*a;
-                      }
-                      bcount=0;
-                  }
-                  a++;
-              }
-          } else {
-              strcpyW(p,*arg);
-              p+=strlenW(*arg);
-          }
-          if (has_space)
-              *p++='"';
-          *p++=' ';
-      }
-      if (p > cmd)
-          p--;  /* remove last space */
-      *p = '\0';
-
-      WINE_TRACE("/c command line: '%s'\n", wine_dbgstr_w(cmd));
+      opt_s |= !may_keep_quotes(cmd);
 
       /* strip first and last quote characters if opt_s; check for invalid
        * executable is done later */
       if (opt_s && *cmd=='\"')
           WCMD_opt_s_strip_quotes(cmd);
+
+      WINE_TRACE("/c command line: '%s'\n", wine_dbgstr_w(cmd));
   }
 
   if (opt_c) {
@@ -2457,7 +2386,7 @@ int wmain (int argc, WCHAR *argvW[])
       WCMD_free_commands(toExecute);
       toExecute = NULL;
 
-      HeapFree(GetProcessHeap(), 0, cmd);
+      HeapFree(GetProcessHeap(), 0, (void*)cmdline);
       return errorlevel;
   }
 
@@ -2552,7 +2481,7 @@ int wmain (int argc, WCHAR *argvW[])
       WCMD_process_commands(toExecute, FALSE, NULL, NULL);
       WCMD_free_commands(toExecute);
       toExecute = NULL;
-      HeapFree(GetProcessHeap(), 0, cmd);
+      HeapFree(GetProcessHeap(), 0, (void*)cmdline);
   }
 
 /*
-- 
1.7.3.4



More information about the wine-patches mailing list