Bug #907: Various system() and backticks problems on Windows

• Status: Closed
• Assignee: -
• Target version: -

[quix (James M. Lawrence)]

=begin

• system("x/y.bat") fails because it is interpreted as option /y being
  passed to command x.

• system("y") fails when y.bat is in the current directory and "." is
  in PATH. "y" succeeds in the console shell window. Similar problems
  occur when passing relative or absolute paths.

• system("c:/x/z", "1") fails to spawn c:/x/z.exe.

• Variables are not expanded:
  system(%{ruby -e "p '%PATH%'"}) #=> "%PATH%"

• Non-existent file in backticks noexist raises Errno::ENOENT.
  On Linux no exception is raised.

Proposed spec:

• http://github.com/quix/alt_system/tree/master/spec/system_spec.rb

Current failures on the spec:

• http://drake.rubyforge.org/tmp/system_spec_kernel.html

A prototype patch which fits the spec:

• http://drake.rubyforge.org/tmp/system_spec_alt.html
• http://github.com/quix/alt_system/tree/master/lib/alt_system/alt_system.rb
  =end

[quix (James M. Lawrence)]

=begin
This was also found by the spec:

• system("a.exe", "1") fails when a.exe is in PATH.

=end

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

=begin
Applied in changeset r20892.
=end

[quix (James M. Lawrence)]

=begin

Latest trunk passes the spec except for the following case: I expected system("echo", "1") to fail since echo only exists as a built-in shell command, while multi-arg system() should not invoke the shell. However I see the lookup on the cmd.exe command list and the join on argv, so presumably the current behavior is intended.

Note there are other popular Windows shells such the PowerShell, so the general strategy of checking for built-in shell commands cannot work.

Also, is a backport to 1.8 planned? This affects Rake.

=end

[nobu (Nobuyoshi Nakada)]

=begin
Hi,

At Tue, 6 Jan 2009 22:31:49 +0900,
James M. Lawrence wrote in [ruby-core:21167]:

Latest trunk passes the spec except for the following case: I
expected system("echo", "1") to fail since echo only exists
as a built-in shell command, while multi-arg system() should
not invoke the shell. However I see the lookup on the
cmd.exe command list and the join on argv, so presumably the
current behavior is intended.

Yes.

Note there are other popular Windows shells such the
PowerShell, so the general strategy of checking for built-in
shell commands cannot work.

Is COMSPEC the PowerShell by the default on such systems?

Also, is a backport to 1.8 planned? This affects Rake.

Could you test the following patch?

Index: io.c

--- io.c (revision 21607)
+++ io.c (working copy)
@@ -3189,5 +3189,5 @@ pipe_open(pstr, pname, mode)
#ifdef _WIN32
retry:

• pid = pipe_exec(pname, rb_io_mode_modenum(mode), &fpr, &fpw);

• pid = rb_w32_pipe_exec(pname, rb_io_mode_modenum(mode), &fpr, &fpw);
  if (pid == -1) { /* exec failed */
  if (errno == EAGAIN) {
  Index: process.c
  ===================================================================
  --- process.c (revision 21607)
  +++ process.c (working copy)
  @@ -1036,5 +1036,5 @@ rb_proc_exec(str)
  #ifdef _WIN32
  before_exec();

• do_spawn(P_OVERLAY, (char *)str);

• rb_w32_spawn(P_OVERLAY, (char *)str, NULL);
  after_exec();
  #else
  @@ -1087,4 +1087,7 @@ rb_proc_exec(str)

#if defined(human68k) || defined(DJGPP) || defined(_WIN32)
+#if defined(_WIN32)
+#define proc_spawn_v(argv, prog) rb_w32_aspawn(P_WAIT, prog, argv)
+#elif defined(human68k) || defined(DJGPP)
static int
proc_spawn_v(argv, prog)
@@ -1129,12 +1132,10 @@ proc_spawn_v(argv, prog)
#endif
before_exec();
-#if defined(_WIN32)

• status = do_aspawn(P_WAIT, prog, argv);
  -#else
  status = spawnv(P_WAIT, prog, argv);
  -#endif

• rb_last_status_set(status == -1 ? 127 : status, 0);
  after_exec();
  return status;
  }
  +#endif

static int
@@ -1149,9 +1150,6 @@ proc_spawn_n(argc, argv, prog)
args = ALLOCA_N(char*, argc + 1);
for (i = 0; i < argc; i++) {

• SafeStringValue(argv[i]);
• args[i] = StringValueCStr(argv[i]);

• args[i] = RSTRING_PTR(argv[i]);
  }

• if (prog)
• SafeStringValue(prog);
  args[i] = (char*) 0;
  if (args[0])
  @@ -1160,16 +1158,15 @@ proc_spawn_n(argc, argv, prog)
  }

-#if !defined(_WIN32)
+#if defined(_WIN32)
+#define proc_spawn(str) rb_w32_spawn(P_WAIT, str, 0)
+#else
static int
-proc_spawn(sv)

• VALUE sv;
  -{
  +proc_spawn(str)
  char *str;
  +{
  char *s, *t;
  char **argv, **a;
  int status;

• SafeStringValue(sv);

• str = s = StringValueCStr(sv);
  for (s = str; *s; s++) {
  if (*s != ' ' && !ISALPHA(s) && strchr("?{}[]<>()~&|\$;'`"\n",s)) {
  @@ -1177,4 +1174,5 @@ proc_spawn(sv)
  before_exec();
  status = shell?spawnl(P_WAIT,shell,"sh","-c",str,(char)NULL):system(str);

•  last_status_set(status == -1 ? 127 : status, 0);
   after_exec();
   return status;
  

@@ -1287,5 +1285,4 @@ rb_f_exec(argc, argv)
prog = RARRAY(tmp)->ptr[0];
argv[0] = RARRAY(tmp)->ptr[1];

• SafeStringValue(prog);
  }
  proc_prepare_args(&earg, argc, argv, prog);
  @@ -1471,100 +1468,66 @@ rb_f_system(argc, argv)
  VALUE *argv;
  {
• int status;
  -#if defined(EMX)

• int status, i;
  +#if defined(EMX) || defined(__VMS)
  VALUE cmd;
  +#else
• volatile VALUE prog = 0;
  +# if !(defined(human68k) || defined(DJGPP) || defined(_WIN32))
• int pid;
• struct rb_exec_arg earg;
• RETSIGTYPE (*chfunc)(int);
  +# endif
  +#endif

+#if !defined(__VMS)
fflush(stdout);
fflush(stderr);
+#endif
if (argc == 0) {
rb_last_status = Qnil;
rb_raise(rb_eArgError, "wrong number of arguments");
}

  if (TYPE(argv[0]) == T_ARRAY) {
if (RARRAY(argv[0])->len != 2) {
    rb_raise(rb_eArgError, "wrong first argument");
}

+#if defined(EMX) || defined(__VMS)
argv[0] = RARRAY(argv[0])->ptr[0];
+#else

• prog = RARRAY(argv[0])->ptr[0];

• argv[0] = RARRAY(argv[0])->ptr[1];
  +#endif

• }

• if (prog && !(argc > 0 && argv[0] == prog)) {

• SafeStringValue(prog);

• StringValueCStr(prog);

• }

• for (i = 0; i < argc; ++i) {

• SafeStringValue(argv[i]);

• StringValueCStr(argv[i]);
  }
  +#if defined(EMX) || defined(__VMS)
  cmd = rb_ary_join(rb_ary_new4(argc, argv), rb_str_new2(" "));

  SafeStringValue(cmd);

• status = do_spawn(RSTRING(cmd)->ptr);
  +# if defined(EMX)

• status = do_spawn(StringValueCStr(cmd));
  last_status_set(status, 0);
  +# else
• status = system(StringValueCStr(cmd));
• last_status_set((status & 0xff) << 8, 0);
  +# endif
  #elif defined(human68k) || defined(DJGPP) || defined(_WIN32)

• volatile VALUE prog = 0;
• fflush(stdout);
• fflush(stderr);
• if (argc == 0) {
• rb_last_status = Qnil;
• rb_raise(rb_eArgError, "wrong number of arguments");
• }
• if (TYPE(argv[0]) == T_ARRAY) {
• if (RARRAY(argv[0])->len != 2) {

•  rb_raise(rb_eArgError, "wrong first argument");
  

• }
• prog = RARRAY(argv[0])->ptr[0];
• argv[0] = RARRAY(argv[0])->ptr[1];
• }
• if (argc == 1 && prog == 0) {
  -#if defined(_WIN32)
• SafeStringValue(argv[0]);
• status = do_spawn(P_WAIT, StringValueCStr(argv[0]));
  -#else
• status = proc_spawn(argv[0]);
  -#endif

• status = proc_spawn(RSTRING_PTR(argv[0]));
  }
  else {
  status = proc_spawn_n(argc, argv, prog);
  }
  -#if !defined(_WIN32)

• last_status_set(status == -1 ? 127 : status, 0);
  -#else
  +# if defined(_WIN32)
  if (status == -1)
  last_status_set(0x7f << 8, 0);
  -#endif
  -#elif defined(__VMS)
• VALUE cmd;
• if (argc == 0) {
• rb_last_status = Qnil;
• rb_raise(rb_eArgError, "wrong number of arguments");
• }
• if (TYPE(argv[0]) == T_ARRAY) {
• if (RARRAY(argv[0])->len != 2) {

•  rb_raise(rb_eArgError, "wrong first argument");
  

• }
• argv[0] = RARRAY(argv[0])->ptr[0];
• }
• cmd = rb_ary_join(rb_ary_new4(argc, argv), rb_str_new2(" "));
• SafeStringValue(cmd);
• status = system(StringValueCStr(cmd));
• last_status_set((status & 0xff) << 8, 0);
  +# endif
  #else
• volatile VALUE prog = 0;
• int pid;
• struct rb_exec_arg earg;
• RETSIGTYPE (*chfunc)(int);
• fflush(stdout);
• fflush(stderr);
• if (argc == 0) {
• rb_last_status = Qnil;
• rb_raise(rb_eArgError, "wrong number of arguments");
• }
• if (TYPE(argv[0]) == T_ARRAY) {
• if (RARRAY(argv[0])->len != 2) {

•  rb_raise(rb_eArgError, "wrong first argument");
  

• }
• prog = RARRAY(argv[0])->ptr[0];
• argv[0] = RARRAY(argv[0])->ptr[1];
• }
  proc_prepare_args(&earg, argc, argv, prog);

Index: win32/win32.c

--- win32/win32.c (revision 21607)
+++ win32/win32.c (working copy)
@@ -28,7 +28,7 @@
#include <wincon.h>
#include <shlobj.h>
+#include <mbstring.h>
#ifdef MINGW32
#include <mswsock.h>
-#include <mbstring.h>
#endif
#include "win32.h"
@@ -47,8 +47,4 @@
#undef setsockopt

-#ifndef bool
-#define bool int
-#endif

#if defined BORLANDC || defined _WIN32_WCE

define _filbuf _fgetc

@@ -86,5 +82,5 @@

static struct ChildRecord *CreateChild(const char *, const char *, SECURITY_ATTRIBUTES *, HANDLE, HANDLE, HANDLE);
-static bool has_redirection(const char *);
+static int has_redirection(const char *);
static void StartSockets ();
static DWORD wait_events(HANDLE event, DWORD timeout);
@@ -390,4 +386,15 @@ exit_handler(void)
}

+static inline char *
+translate_char(char *p, int from, int to)
+{

• while (*p) {
• if ((unsigned char)*p == from)

•  *p = to;
  

• p = CharNext(p);
• }
• return p;
  +}

#ifndef CSIDL_PROFILE
#define CSIDL_PROFILE 40
@@ -435,9 +442,5 @@ init_env(void)
}
if (f) {

•  char *p = env;
  

•  while (*p) {
  

•  if (*p == '\\') *p = '/';
  

•  p = CharNext(p);
  

•  }
  

•  char *p = translate_char(env, '\\', '/');
   if (p - env == 2 && env[1] == ':') {
   *p++ = '/';
  

@@ -643,8 +646,10 @@ is_command_com(const char *interp)
}

+static int internal_cmd_match(const char *cmdname, int nt);
+
static int
is_internal_cmd(const char *cmd, int nt)
{

• char cmdname[9], *b = cmdname, c, **nm;

• char cmdname[9], *b = cmdname, c;

  do {
  @@ -666,4 +671,12 @@ is_internal_cmd(const char *cmd, int nt)
  }
  *b = 0;

• return internal_cmd_match(cmdname, nt);
  +}

+static int
+internal_cmd_match(const char *cmdname, int nt)
+{

• char **nm;
• nm = bsearch(cmdname, szInternalCmds,
  sizeof(szInternalCmds) / sizeof(*szInternalCmds),
  @@ -675,5 +688,4 @@ is_internal_cmd(const char *cmd, int nt)
  }

SOCKET
rb_w32_get_osfhandle(int fh)
@@ -683,5 +695,5 @@ rb_w32_get_osfhandle(int fh)

rb_pid_t
-pipe_exec(const char *cmd, int mode, FILE **fpr, FILE **fpw)
+rb_w32_pipe_exec(const char *cmd, int mode, FILE **fpr, FILE *fpw)
{
struct ChildRecord child;
@@ -821,58 +833,10 @@ pipe_exec(const char *cmd, int mode, FIL
extern VALUE rb_last_status;

-int
-do_spawn(int mode, const char *cmd)
-{

• struct ChildRecord *child;
• DWORD exitcode;
• switch (mode) {

•  case P_WAIT:
  

•  case P_NOWAIT:
  

•  case P_OVERLAY:
  

• break;

•  default:
  

• errno = EINVAL;
• return -1;
• }
• child = CreateChild(cmd, NULL, NULL, NULL, NULL, NULL);
• if (!child) {
• return -1;
• }
• switch (mode) {

•  case P_WAIT:
  

• rb_syswait(child->pid);
• return NUM2INT(rb_last_status);

•  case P_NOWAIT:
  

• return child->pid;

•  case P_OVERLAY:
  

• WaitForSingleObject(child->hProcess, INFINITE);
• GetExitCodeProcess(child->hProcess, &exitcode);
• CloseChildHandle(child);
• _exit(exitcode);

•  default:
  

• return -1; /* not reached */
• }
  -}

-int
-do_aspawn(int mode, const char *prog, char **argv)
+static int
+argv_size(char *const *argv, BOOL escape)
{

• char *cmd, *p, *q, *s, **t;

• const char *p;
• char *const *t;
  int len, n, bs, quote;

• struct ChildRecord *child;

• DWORD exitcode;

• switch (mode) {

•  case P_WAIT:
  

•  case P_NOWAIT:
  

•  case P_OVERLAY:
  

• break;

•  default:
  

• errno = EINVAL;

• return -1;

• }

  for (t = argv, len = 0; *t; t++) {
  @@ -882,4 +846,8 @@ do_aspawn(int mode, const char *prog, ch
  ++bs;
  break;

•    case '<': case '>': case '|': case '^':
  

•  bs = 0;
  

•  if (escape && !quote) n++;
  

•  break;
     case '"':
   n += bs + 1; bs = 0;
  

@@ -895,7 +863,16 @@ do_aspawn(int mode, const char *prog, ch
}
len += p - *t + n + 1;

• if (quote) len += 2;

• if (p - *t == 0 || quote) len += 2;
  }

• cmd = ALLOCA_N(char, len);

• return len;
  +}

+static char *
+join_argv(char *cmd, char *const *argv, BOOL escape)
+{

• const char *p, *s;
• char *q, *const *t;
• int n, bs, quote;
• for (t = argv, q = cmd; p = *t; t++) {
  quote = 0;
  @@ -914,4 +891,10 @@ do_aspawn(int mode, const char *prog, ch
  memset(q, '\', ++bs); q += bs; bs = 0;
  break;

•    case '<': case '>': case '|': case '^':
  

•  if (escape && !quote) {
  

•      memcpy(q, s, n = p - s); q += n; s = p;
  

•      *q++ = '^';
  

•      break;
  

•  }
     default:
   bs = 0;
  

@@ -927,6 +910,35 @@ do_aspawn(int mode, const char *prog, ch
if (q > cmd) --q;
*q = '\0';

• return cmd;
  +}

+#ifdef HAVE_SYS_PARAM_H
+# include <sys/param.h>
+#else
+# define MAXPATHLEN 512
+#endif
+
+#define STRNDUPA(ptr, src, len) \

• (((char *)memcpy(((ptr) = ALLOCA_N(char, (len) + 1)), (src), (len)))[len] = 0)

+static int
+check_spawn_mode(int mode)
+{

• switch (mode) {

•  case P_WAIT:
  

•  case P_NOWAIT:
  

•  case P_OVERLAY:
  

• return 0;

•  default:
  

• errno = EINVAL;
• return -1;
• }
  +}

+static rb_pid_t
+child_result(struct ChildRecord *child, int mode)
+{

• DWORD exitcode;

• child = CreateChild(cmd, prog, NULL, NULL, NULL, NULL);
  if (!child) {
  return -1;
  @@ -958,7 +970,5 @@ CreateChild(const char *cmd, const char
  PROCESS_INFORMATION aProcessInformation;
  SECURITY_ATTRIBUTES sa;

• const char *shell;
  struct ChildRecord *child;

• char *p = NULL;

  if (!cmd && !prog) {
  @@ -1007,4 +1017,37 @@ CreateChild(const char *cmd, const char
  dwCreationFlags = (NORMAL_PRIORITY_CLASS);

• RUBY_CRITICAL({
• fRet = CreateProcess(prog, (char *)cmd, psa, psa,

•  	     psa->bInheritHandle, dwCreationFlags, NULL, NULL,
  

•  	     &aStartupInfo, &aProcessInformation);
  

• errno = map_errno(GetLastError());
• });
• if (!fRet) {
• child->pid = 0; /* release the slot */
• return NULL;
• }
• CloseHandle(aProcessInformation.hThread);
• child->hProcess = aProcessInformation.hProcess;
• child->pid = (rb_pid_t)aProcessInformation.dwProcessId;
• if (!IsWinNT()) {
• /* On Win9x, make pid positive similarly to cygwin and perl */
• child->pid = -child->pid;
• }
• return child;
  +}

+rb_pid_t
+rb_w32_spawn(int mode, const char *cmd, const char *prog)
+{

• char *p = NULL;
• const char *shell = NULL;
• if (check_spawn_mode(mode)) return -1;
• if (prog) {
  if (!(p = dln_find_exe(prog, NULL))) {
  @@ -1014,21 +1057,9 @@ CreateChild(const char *cmd, const char
  else {
  int redir = -1;

• int len = 0;
  int nt;
  while (ISSPACE(*cmd)) cmd++;
• for (prog = cmd; *prog; prog = CharNext(prog)) {

•  if (ISSPACE(*prog)) {
  

•  len = prog - cmd;
  

•  do ++prog; while (ISSPACE(*prog));
  

•  if (!*prog--) break;
  

•  }
  

•  else {
  

•  len = 0;
  

•  }
  

• }
• if (!len) len = strlen(cmd);
  if ((shell = getenv("RUBYSHELL")) && (redir = has_redirection(cmd))) {

•  char *tmp = ALLOCA_N(char, strlen(shell) + len + sizeof(" -c ") + 2);
  

•  sprintf(tmp, "%s -c \"%.*s\"", shell, len, cmd);
  

•  char *tmp = ALLOCA_N(char, strlen(shell) + strlen(cmd) + sizeof(" -c ") + 2);
  

•  sprintf(tmp, "%s -c \"%s\"", shell, cmd);
   cmd = tmp;
  

  }
  @@ -1037,10 +1068,30 @@ CreateChild(const char *cmd, const char
  (redir < 0 ? has_redirection(cmd) : redir) ||
  is_internal_cmd(cmd, nt))) {

•  char *tmp = ALLOCA_N(char, strlen(shell) + len + sizeof(" /c ")
  

•  char *tmp = ALLOCA_N(char, strlen(shell) + strlen(cmd) + sizeof(" /c ")
   		 + (nt ? 2 : 0));
  

•  sprintf(tmp, nt ? "%s /c \"%.*s\"" : "%s /c %.*s", shell, len, cmd);
  

•  sprintf(tmp, nt ? "%s /c \"%s\"" : "%s /c %s", shell, cmd);
   cmd = tmp;
  

  }
  else {

•  int len = 0;
  

•  if (*cmd == '"') {
  

•  for (prog = cmd + 1; *prog && *prog != '"'; prog = CharNext(prog));
  

•  len = prog - cmd - 1;
  

•  STRNDUPA(p, cmd + 1, len);
  

•  p = dln_find_exe(p, NULL);
  

•  if (p) goto command_found;
  

•  }
  

•  for (prog = cmd; *prog; prog = CharNext(prog)) {
  

•  if (ISSPACE(*prog)) {
  

•      len = prog - cmd;
  

•      do ++prog; while (ISSPACE(*prog));
  

•      break;
  

•  }
  

•  else {
  

•      len = 0;
  

•  }
  

•  }
  

•  if (!len) len = strlen(cmd);
  

•  shell = NULL;
   prog = cmd;
  

@@ -1050,18 +1101,14 @@ CreateChild(const char *cmd, const char
break;
}

•  if (strchr(".:*?\"/\\", *prog)) {
  

•  if (strchr(":*?\"/\\", *prog)) {
       if (cmd[len]) {
  

•  	char *tmp = ALLOCA_N(char, len + 1);
  

•  	memcpy(tmp, cmd, len);
  

•  	tmp[len] = 0;
  

•  	cmd = tmp;
  

•  	STRNDUPA(p, cmd, len);
       }
  

•      p = dln_find_exe(p ? p : cmd, NULL);
       break;
   }
   if (ISSPACE(*prog) || strchr("<>|", *prog)) {
       len = prog - cmd;
  

•      p = ALLOCA_N(char, len + 1);
  

•      memcpy(p, cmd, len);
  

•      p[len] = 0;
  

•      STRNDUPA(p, cmd, len);
       p = dln_find_exe(p, NULL);
       break;
  

@@ -1072,35 +1119,73 @@ CreateChild(const char *cmd, const char
}
if (p) {

•  command_found:
  

  shell = p;

• while (*p) {

•  if ((unsigned char)*p == '/')
  

•  *p = '\\';
  

•  p = CharNext(p);
  

• }

• translate_char(p, '/', '\');
  }

• RUBY_CRITICAL({
• fRet = CreateProcess(shell, (char *)cmd, psa, psa,

•  	     psa->bInheritHandle, dwCreationFlags, NULL, NULL,
  

•  	     &aStartupInfo, &aProcessInformation);
  

• errno = map_errno(GetLastError());
• });
• if (!fRet) {
• child->pid = 0; /* release the slot */
• return NULL;
• }

• return child_result(CreateChild(cmd, shell, NULL, NULL, NULL, NULL), mode);
  +}

• CloseHandle(aProcessInformation.hThread);
  +rb_pid_t
  +rb_w32_aspawn(int mode, const char *prog, char *const *argv)
  +{

• int len, differ = 0, c_switch = 0;
• BOOL ntcmd = FALSE, tmpnt;
• const char *shell;
• char *cmd;

• child->hProcess = aProcessInformation.hProcess;
• child->pid = (rb_pid_t)aProcessInformation.dwProcessId;

• if (check_spawn_mode(mode)) return -1;

• if (!IsWinNT()) {
• /* On Win9x, make pid positive similarly to cygwin and perl */
• child->pid = -child->pid;

• if (!prog) prog = argv[0];
• if ((shell = getenv("COMSPEC")) &&
• internal_cmd_match(prog, tmpnt = !is_command_com(shell))) {
• ntcmd = tmpnt;
• prog = shell;
• c_switch = 1;
• differ = 1;
• }
• else if ((cmd = dln_find_exe(prog, NULL))) {
• if (cmd == prog) {

•  cmd = ALLOCA_N(char, strlen(cmd));
  

•  strcpy(cmd, prog);
  

• }
• translate_char(cmd, '/', '\');
• prog = cmd;
• argv++;
• differ = 1;
• }
• else if (strchr(prog, '/')) {
• cmd = ALLOCA_N(char, strlen(cmd));
• strcpy(cmd, prog);
• translate_char(cmd, '/', '\');
• argv++;
• differ = 1;
• }
• if (differ) {
• char *progs[2];
• int cmdlen;
• progs[0] = (char *)prog;
• progs[1] = NULL;
• len = argv_size(progs, ntcmd);
• cmdlen = len - 1;
• if (c_switch) len += 3;
• if (argv[0]) len += argv_size(argv, ntcmd);
• cmd = ALLOCA_N(char, len);
• join_argv(cmd, progs, ntcmd);
• if (c_switch) {

•  memcpy(cmd + cmdlen, " /c", 4);
  

•  cmdlen += 3;
  

• }
• if (argv[0]) {

•  cmd[cmdlen++] = ' ';
  

•  join_argv(cmd + cmdlen, argv, ntcmd);
  

• }
  }

• return child;

• else {
• len = argv_size(argv, FALSE);
• cmd = ALLOCA_N(char, len);
• join_argv(cmd, argv, FALSE);
• }
• return child_result(CreateChild(cmd, prog, NULL, NULL, NULL, NULL), mode);
  }

@@ -1139,10 +1224,4 @@ insert(const char *path, VALUE vinfo)
}

-#ifdef HAVE_SYS_PARAM_H
-# include <sys/param.h>
-#else
-# define MAXPATHLEN 512
-#endif

static NtCmdLineElement **
@@ -1157,5 +1236,5 @@ cmdglob(NtCmdLineElement *patt, NtCmdLin
if (!(buf = malloc(patt->len + 1))) return 0;

• strncpy (buf, patt->str, patt->len);

• memcpy(buf, patt->str, patt->len + 1);
  buf[patt->len] = '\0';
  for (p = buf; *p; p = CharNext(p))
  @@ -1178,5 +1257,5 @@ cmdglob(NtCmdLineElement *patt, NtCmdLin
  //

-static bool
+static int
has_redirection(const char *cmd)
{
@@ -1208,4 +1287,10 @@ has_redirection(const char *cmd)
break;

• case '%':

•  if (*++ptr != '_' && !ISALPHA(*ptr)) break;
  

•  while (*++ptr == '_' || ISALNUM(*ptr));
  

•  if (*ptr++ == '%') return TRUE;
  

•  break;
  

• case '\':
  ptr++;
  Index: win32/win32.h
  ===================================================================
  --- win32/win32.h (revision 21607)
  +++ win32/win32.h (working copy)
  @@ -139,5 +139,5 @@ extern DWORD rb_w32_osid(void);
  #define stat(path,st) rb_w32_stat(path,st)
  #undef execv
  -#define execv(path,argv) do_aspawn(P_OVERLAY,path,argv)
  +#define execv(path,argv) rb_w32_aspawn(P_OVERLAY,path,argv)
  #if !defined(BORLANDC) && !defined(_WIN32_WCE)
  #undef isatty
  @@ -161,5 +161,5 @@ struct timezone {
  extern void NtInitialize(int *, char ***);
  extern int rb_w32_cmdvector(const char *, char ***);
  -extern rb_pid_t pipe_exec(const char *, int, FILE **, FILE **);
  +extern rb_pid_t rb_w32_pipe_exec(const char *, int, FILE **, FILE **);
  extern int flock(int fd, int oper);
  extern int rb_w32_accept(int, struct sockaddr *, int *);
  @@ -207,6 +207,6 @@ extern int link(char *, char *);
  extern int gettimeofday(struct timeval *, struct timezone *);
  extern rb_pid_t waitpid (rb_pid_t, int *, int);
  -extern int do_spawn(int, const char *);
  -extern int do_aspawn(int, const char *, char **);
  +extern rb_pid_t rb_w32_spawn(int, const char *, const char *prog);
  +extern rb_pid_t rb_w32_aspawn(int, const char *, char *const *);
  extern int kill(int, int);
  extern int fcntl(int, int, ...);
  

--
Nobu Nakada

=end

[jredville (Jim Deville)]

=begin
COMSPEC is still cmd by default. Win7 doesn't change that even though Powershell is in the box. Installing Powershell doesn't change it either.

JD

-----Original Message-----
From: Nobuyoshi Nakada [e-mail]
Sent: January 18, 2009 12:46 PM
To: [e-mail] [e-mail]
Subject: [ruby-core:21417] Re: [Bug #907] Various system() and backticks problems on Windows

Hi,

At Tue, 6 Jan 2009 22:31:49 +0900,
James M. Lawrence wrote in [ruby-core:21167]:

Latest trunk passes the spec except for the following case: I
expected system("echo", "1") to fail since echo only exists
as a built-in shell command, while multi-arg system() should
not invoke the shell. However I see the lookup on the
cmd.exe command list and the join on argv, so presumably the
current behavior is intended.

Yes.

Note there are other popular Windows shells such the
PowerShell, so the general strategy of checking for built-in
shell commands cannot work.

Is COMSPEC the PowerShell by the default on such systems?

Also, is a backport to 1.8 planned? This affects Rake.

Could you test the following patch?

Index: io.c

--- io.c (revision 21607)
+++ io.c (working copy)
@@ -3189,5 +3189,5 @@ pipe_open(pstr, pname, mode)
#ifdef _WIN32
retry:

• pid = pipe_exec(pname, rb_io_mode_modenum(mode), &fpr, &fpw);

• pid = rb_w32_pipe_exec(pname, rb_io_mode_modenum(mode), &fpr, &fpw);
  if (pid == -1) { /* exec failed */
  if (errno == EAGAIN) {
  Index: process.c
  ===================================================================
  --- process.c (revision 21607)
  +++ process.c (working copy)
  @@ -1036,5 +1036,5 @@ rb_proc_exec(str)
  #ifdef _WIN32
  before_exec();

• do_spawn(P_OVERLAY, (char *)str);

• rb_w32_spawn(P_OVERLAY, (char *)str, NULL);
  after_exec();
  #else
  @@ -1087,4 +1087,7 @@ rb_proc_exec(str)

#if defined(human68k) || defined(DJGPP) || defined(_WIN32)
+#if defined(_WIN32)
+#define proc_spawn_v(argv, prog) rb_w32_aspawn(P_WAIT, prog, argv)
+#elif defined(human68k) || defined(DJGPP)
static int
proc_spawn_v(argv, prog)
@@ -1129,12 +1132,10 @@ proc_spawn_v(argv, prog)
#endif
before_exec();
-#if defined(_WIN32)

• status = do_aspawn(P_WAIT, prog, argv);
  -#else
  status = spawnv(P_WAIT, prog, argv);
  -#endif

• rb_last_status_set(status == -1 ? 127 : status, 0);
  after_exec();
  return status;
  }
  +#endif

static int
@@ -1149,9 +1150,6 @@ proc_spawn_n(argc, argv, prog)
args = ALLOCA_N(char*, argc + 1);
for (i = 0; i < argc; i++) {

•   SafeStringValue(argv[i]);
  

•   args[i] = StringValueCStr(argv[i]);
  

•   args[i] = RSTRING_PTR(argv[i]);
  

  }

• if (prog)

•   SafeStringValue(prog);
  

  args[i] = (char*) 0;
  if (args[0])
  @@ -1160,16 +1158,15 @@ proc_spawn_n(argc, argv, prog)
  }

-#if !defined(_WIN32)
+#if defined(_WIN32)
+#define proc_spawn(str) rb_w32_spawn(P_WAIT, str, 0)
+#else
static int
-proc_spawn(sv)

• VALUE sv;
  -{
  +proc_spawn(str)
  char *str;
  +{
  char *s, *t;
  char **argv, **a;
  int status;

• SafeStringValue(sv);

• str = s = StringValueCStr(sv);
  for (s = str; *s; s++) {
  if (*s != ' ' && !ISALPHA(s) && strchr("?{}[]<>()~&|\$;'`"\n",s)) {
  @@ -1177,4 +1174,5 @@ proc_spawn(sv)
  before_exec();
  status = shell?spawnl(P_WAIT,shell,"sh","-c",str,(char)NULL):system(str);

•       last_status_set(status == -1 ? 127 : status, 0);
        after_exec();
        return status;
  

@@ -1287,5 +1285,4 @@ rb_f_exec(argc, argv)
prog = RARRAY(tmp)->ptr[0];
argv[0] = RARRAY(tmp)->ptr[1];

•   SafeStringValue(prog);
  

  }
  proc_prepare_args(&earg, argc, argv, prog);
  @@ -1471,100 +1468,66 @@ rb_f_system(argc, argv)
  VALUE *argv;
  {
• int status;
  -#if defined(EMX)

• int status, i;
  +#if defined(EMX) || defined(__VMS)
  VALUE cmd;
  +#else
• volatile VALUE prog = 0;
  +# if !(defined(human68k) || defined(DJGPP) || defined(_WIN32))
• int pid;
• struct rb_exec_arg earg;
• RETSIGTYPE (*chfunc)(int);
  +# endif
  +#endif

+#if !defined(__VMS)
fflush(stdout);
fflush(stderr);
+#endif
if (argc == 0) {
rb_last_status = Qnil;
rb_raise(rb_eArgError, "wrong number of arguments");
}

  if (TYPE(argv[0]) == T_ARRAY) {
     if (RARRAY(argv[0])->len != 2) {
         rb_raise(rb_eArgError, "wrong first argument");
     }

+#if defined(EMX) || defined(__VMS)
argv[0] = RARRAY(argv[0])->ptr[0];
+#else

•   prog = RARRAY(argv[0])->ptr[0];
  

•   argv[0] = RARRAY(argv[0])->ptr[1];
  

+#endif

• }

• if (prog && !(argc > 0 && argv[0] == prog)) {

•   SafeStringValue(prog);
  

•   StringValueCStr(prog);
  

• }

• for (i = 0; i < argc; ++i) {

•   SafeStringValue(argv[i]);
  

•   StringValueCStr(argv[i]);
  

  }
  +#if defined(EMX) || defined(__VMS)
  cmd = rb_ary_join(rb_ary_new4(argc, argv), rb_str_new2(" "));

  SafeStringValue(cmd);

• status = do_spawn(RSTRING(cmd)->ptr);
  +# if defined(EMX)

• status = do_spawn(StringValueCStr(cmd));
  last_status_set(status, 0);
  +# else
• status = system(StringValueCStr(cmd));
• last_status_set((status & 0xff) << 8, 0);
  +# endif
  #elif defined(human68k) || defined(DJGPP) || defined(_WIN32)

• volatile VALUE prog = 0;
• fflush(stdout);
• fflush(stderr);
• if (argc == 0) {

•   rb_last_status = Qnil;
  

•   rb_raise(rb_eArgError, "wrong number of arguments");
  

• }
• if (TYPE(argv[0]) == T_ARRAY) {

•   if (RARRAY(argv[0])->len != 2) {
  

•       rb_raise(rb_eArgError, "wrong first argument");
  

•   }
  

•   prog = RARRAY(argv[0])->ptr[0];
  

•   argv[0] = RARRAY(argv[0])->ptr[1];
  

• }
• if (argc == 1 && prog == 0) {
  -#if defined(_WIN32)

•   SafeStringValue(argv[0]);
  

•   status = do_spawn(P_WAIT, StringValueCStr(argv[0]));
  

-#else

•   status = proc_spawn(argv[0]);
  

-#endif

•   status = proc_spawn(RSTRING_PTR(argv[0]));
  

  }
  else {
  status = proc_spawn_n(argc, argv, prog);
  }
  -#if !defined(_WIN32)

• last_status_set(status == -1 ? 127 : status, 0);
  -#else
  +# if defined(_WIN32)
  if (status == -1)
  last_status_set(0x7f << 8, 0);
  -#endif
  -#elif defined(__VMS)
• VALUE cmd;
• if (argc == 0) {

•   rb_last_status = Qnil;
  

•   rb_raise(rb_eArgError, "wrong number of arguments");
  

• }
• if (TYPE(argv[0]) == T_ARRAY) {

•   if (RARRAY(argv[0])->len != 2) {
  

•       rb_raise(rb_eArgError, "wrong first argument");
  

•   }
  

•   argv[0] = RARRAY(argv[0])->ptr[0];
  

• }
• cmd = rb_ary_join(rb_ary_new4(argc, argv), rb_str_new2(" "));
• SafeStringValue(cmd);
• status = system(StringValueCStr(cmd));
• last_status_set((status & 0xff) << 8, 0);
  +# endif
  #else
• volatile VALUE prog = 0;
• int pid;
• struct rb_exec_arg earg;
• RETSIGTYPE (*chfunc)(int);
• fflush(stdout);
• fflush(stderr);
• if (argc == 0) {

•   rb_last_status = Qnil;
  

•   rb_raise(rb_eArgError, "wrong number of arguments");
  

• }
• if (TYPE(argv[0]) == T_ARRAY) {

•   if (RARRAY(argv[0])->len != 2) {
  

•       rb_raise(rb_eArgError, "wrong first argument");
  

•   }
  

•   prog = RARRAY(argv[0])->ptr[0];
  

•   argv[0] = RARRAY(argv[0])->ptr[1];
  

• }
  proc_prepare_args(&earg, argc, argv, prog);

Index: win32/win32.c

--- win32/win32.c (revision 21607)
+++ win32/win32.c (working copy)
@@ -28,7 +28,7 @@
#include <wincon.h>
#include <shlobj.h>
+#include <mbstring.h>
#ifdef MINGW32
#include <mswsock.h>
-#include <mbstring.h>
#endif
#include "win32.h"
@@ -47,8 +47,4 @@
#undef setsockopt

-#ifndef bool
-#define bool int
-#endif

#if defined BORLANDC || defined _WIN32_WCE

define _filbuf _fgetc

@@ -86,5 +82,5 @@

static struct ChildRecord *CreateChild(const char *, const char *, SECURITY_ATTRIBUTES *, HANDLE, HANDLE, HANDLE);
-static bool has_redirection(const char *);
+static int has_redirection(const char *);
static void StartSockets ();
static DWORD wait_events(HANDLE event, DWORD timeout);
@@ -390,4 +386,15 @@ exit_handler(void)
}

+static inline char *
+translate_char(char *p, int from, int to)
+{

• while (*p) {

•   if ((unsigned char)*p == from)
  

•       *p = to;
  

•   p = CharNext(p);
  

• }
• return p;
  +}

#ifndef CSIDL_PROFILE
#define CSIDL_PROFILE 40
@@ -435,9 +442,5 @@ init_env(void)
}
if (f) {

•       char *p = env;
  

•       while (*p) {
  

•           if (*p == '\\') *p = '/';
  

•           p = CharNext(p);
  

•       }
  

•       char *p = translate_char(env, '\\', '/');
        if (p - env == 2 && env[1] == ':') {
            *p++ = '/';
  

@@ -643,8 +646,10 @@ is_command_com(const char *interp)
}

+static int internal_cmd_match(const char *cmdname, int nt);
+
static int
is_internal_cmd(const char *cmd, int nt)
{

• char cmdname[9], *b = cmdname, c, **nm;

• char cmdname[9], *b = cmdname, c;

  do {
  @@ -666,4 +671,12 @@ is_internal_cmd(const char *cmd, int nt)
  }
  *b = 0;

• return internal_cmd_match(cmdname, nt);
  +}

+static int
+internal_cmd_match(const char *cmdname, int nt)
+{

• char **nm;
• nm = bsearch(cmdname, szInternalCmds,
  sizeof(szInternalCmds) / sizeof(*szInternalCmds),
  @@ -675,5 +688,4 @@ is_internal_cmd(const char *cmd, int nt)
  }

SOCKET
rb_w32_get_osfhandle(int fh)
@@ -683,5 +695,5 @@ rb_w32_get_osfhandle(int fh)

rb_pid_t
-pipe_exec(const char *cmd, int mode, FILE **fpr, FILE **fpw)
+rb_w32_pipe_exec(const char *cmd, int mode, FILE **fpr, FILE *fpw)
{
struct ChildRecord child;
@@ -821,58 +833,10 @@ pipe_exec(const char *cmd, int mode, FIL
extern VALUE rb_last_status;

-int
-do_spawn(int mode, const char *cmd)
-{

• struct ChildRecord *child;
• DWORD exitcode;
• switch (mode) {

•  case P_WAIT:
  

•  case P_NOWAIT:
  

•  case P_OVERLAY:
  

•   break;
  

•  default:
  

•   errno = EINVAL;
  

•   return -1;
  

• }
• child = CreateChild(cmd, NULL, NULL, NULL, NULL, NULL);
• if (!child) {

•   return -1;
  

• }
• switch (mode) {

•  case P_WAIT:
  

•   rb_syswait(child->pid);
  

•   return NUM2INT(rb_last_status);
  

•  case P_NOWAIT:
  

•   return child->pid;
  

•  case P_OVERLAY:
  

•   WaitForSingleObject(child->hProcess, INFINITE);
  

•   GetExitCodeProcess(child->hProcess, &exitcode);
  

•   CloseChildHandle(child);
  

•   _exit(exitcode);
  

•  default:
  

•   return -1;      /* not reached */
  

• }
  -}

-int
-do_aspawn(int mode, const char *prog, char **argv)
+static int
+argv_size(char *const *argv, BOOL escape)
{

• char *cmd, *p, *q, *s, **t;

• const char *p;
• char *const *t;
  int len, n, bs, quote;

• struct ChildRecord *child;

• DWORD exitcode;

• switch (mode) {

•  case P_WAIT:
  

•  case P_NOWAIT:
  

•  case P_OVERLAY:
  

•   break;
  

•  default:
  

•   errno = EINVAL;
  

•   return -1;
  

• }

  for (t = argv, len = 0; *t; t++) {
  @@ -882,4 +846,8 @@ do_aspawn(int mode, const char *prog, ch
  ++bs;
  break;

•         case '<': case '>': case '|': case '^':
  

•           bs = 0;
  

•           if (escape && !quote) n++;
  

•           break;
          case '"':
            n += bs + 1; bs = 0;
  

@@ -895,7 +863,16 @@ do_aspawn(int mode, const char *prog, ch
}
len += p - *t + n + 1;

•   if (quote) len += 2;
  

•   if (p - *t == 0 || quote) len += 2;
  

  }

• cmd = ALLOCA_N(char, len);

• return len;
  +}

+static char *
+join_argv(char *cmd, char *const *argv, BOOL escape)
+{

• const char *p, *s;
• char *q, *const *t;
• int n, bs, quote;
• for (t = argv, q = cmd; p = *t; t++) {
  quote = 0;
  @@ -914,4 +891,10 @@ do_aspawn(int mode, const char *prog, ch
  memset(q, '\', ++bs); q += bs; bs = 0;
  break;

•         case '<': case '>': case '|': case '^':
  

•           if (escape && !quote) {
  

•               memcpy(q, s, n = p - s); q += n; s = p;
  

•               *q++ = '^';
  

•               break;
  

•           }
          default:
            bs = 0;
  

@@ -927,6 +910,35 @@ do_aspawn(int mode, const char *prog, ch
if (q > cmd) --q;
*q = '\0';

• return cmd;
  +}

+#ifdef HAVE_SYS_PARAM_H
+# include <sys/param.h>
+#else
+# define MAXPATHLEN 512
+#endif
+
+#define STRNDUPA(ptr, src, len) \

• (((char *)memcpy(((ptr) = ALLOCA_N(char, (len) + 1)), (src), (len)))[len] = 0)

+static int
+check_spawn_mode(int mode)
+{

• switch (mode) {

•  case P_WAIT:
  

•  case P_NOWAIT:
  

•  case P_OVERLAY:
  

•   return 0;
  

•  default:
  

•   errno = EINVAL;
  

•   return -1;
  

• }
  +}

+static rb_pid_t
+child_result(struct ChildRecord *child, int mode)
+{

• DWORD exitcode;

• child = CreateChild(cmd, prog, NULL, NULL, NULL, NULL);
  if (!child) {
  return -1;
  @@ -958,7 +970,5 @@ CreateChild(const char *cmd, const char
  PROCESS_INFORMATION aProcessInformation;
  SECURITY_ATTRIBUTES sa;

• const char *shell;
  struct ChildRecord *child;

• char *p = NULL;

  if (!cmd && !prog) {
  @@ -1007,4 +1017,37 @@ CreateChild(const char *cmd, const char
  dwCreationFlags = (NORMAL_PRIORITY_CLASS);

• RUBY_CRITICAL({

•   fRet = CreateProcess(prog, (char *)cmd, psa, psa,
  

•                        psa->bInheritHandle, dwCreationFlags, NULL, NULL,
  

•                        &aStartupInfo, &aProcessInformation);
  

•   errno = map_errno(GetLastError());
  

• });
• if (!fRet) {

•   child->pid = 0;         /* release the slot */
  

•   return NULL;
  

• }
• CloseHandle(aProcessInformation.hThread);
• child->hProcess = aProcessInformation.hProcess;
• child->pid = (rb_pid_t)aProcessInformation.dwProcessId;
• if (!IsWinNT()) {

•   /* On Win9x, make pid positive similarly to cygwin and perl */
  

•   child->pid = -child->pid;
  

• }
• return child;
  +}

+rb_pid_t
+rb_w32_spawn(int mode, const char *cmd, const char *prog)
+{

• char *p = NULL;
• const char *shell = NULL;
• if (check_spawn_mode(mode)) return -1;
• if (prog) {
  if (!(p = dln_find_exe(prog, NULL))) {
  @@ -1014,21 +1057,9 @@ CreateChild(const char *cmd, const char
  else {
  int redir = -1;

•   int len = 0;
    int nt;
    while (ISSPACE(*cmd)) cmd++;
  

•   for (prog = cmd; *prog; prog = CharNext(prog)) {
  

•       if (ISSPACE(*prog)) {
  

•           len = prog - cmd;
  

•           do ++prog; while (ISSPACE(*prog));
  

•           if (!*prog--) break;
  

•       }
  

•       else {
  

•           len = 0;
  

•       }
  

•   }
  

•   if (!len) len = strlen(cmd);
    if ((shell = getenv("RUBYSHELL")) && (redir = has_redirection(cmd))) {
  

•       char *tmp = ALLOCA_N(char, strlen(shell) + len + sizeof(" -c ") + 2);
  

•       sprintf(tmp, "%s -c \"%.*s\"", shell, len, cmd);
  

•       char *tmp = ALLOCA_N(char, strlen(shell) + strlen(cmd) + sizeof(" -c ") + 2);
  

•       sprintf(tmp, "%s -c \"%s\"", shell, cmd);
        cmd = tmp;
    }
  

@@ -1037,10 +1068,30 @@ CreateChild(const char *cmd, const char
(redir < 0 ? has_redirection(cmd) : redir) ||
is_internal_cmd(cmd, nt))) {

•       char *tmp = ALLOCA_N(char, strlen(shell) + len + sizeof(" /c ")
  

•       char *tmp = ALLOCA_N(char, strlen(shell) + strlen(cmd) + sizeof(" /c ")
                             + (nt ? 2 : 0));
  

•       sprintf(tmp, nt ? "%s /c \"%.*s\"" : "%s /c %.*s", shell, len, cmd);
  

•       sprintf(tmp, nt ? "%s /c \"%s\"" : "%s /c %s", shell, cmd);
        cmd = tmp;
    }
    else {
  

•       int len = 0;
  

•       if (*cmd == '"') {
  

•           for (prog = cmd + 1; *prog && *prog != '"'; prog = CharNext(prog));
  

•           len = prog - cmd - 1;
  

•           STRNDUPA(p, cmd + 1, len);
  

•           p = dln_find_exe(p, NULL);
  

•           if (p) goto command_found;
  

•       }
  

•       for (prog = cmd; *prog; prog = CharNext(prog)) {
  

•           if (ISSPACE(*prog)) {
  

•               len = prog - cmd;
  

•               do ++prog; while (ISSPACE(*prog));
  

•               break;
  

•           }
  

•           else {
  

•               len = 0;
  

•           }
  

•       }
  

•       if (!len) len = strlen(cmd);
  

•       shell = NULL;
        prog = cmd;
  

@@ -1050,18 +1101,14 @@ CreateChild(const char *cmd, const char
break;
}

•           if (strchr(".:*?\"/\\", *prog)) {
  

•           if (strchr(":*?\"/\\", *prog)) {
                if (cmd[len]) {
  

•                   char *tmp = ALLOCA_N(char, len + 1);
  

•                   memcpy(tmp, cmd, len);
  

•                   tmp[len] = 0;
  

•                   cmd = tmp;
  

•                   STRNDUPA(p, cmd, len);
                }
  

•               p = dln_find_exe(p ? p : cmd, NULL);
                break;
            }
            if (ISSPACE(*prog) || strchr("<>|", *prog)) {
                len = prog - cmd;
  

•               p = ALLOCA_N(char, len + 1);
  

•               memcpy(p, cmd, len);
  

•               p[len] = 0;
  

•               STRNDUPA(p, cmd, len);
                p = dln_find_exe(p, NULL);
                break;
  

@@ -1072,35 +1119,73 @@ CreateChild(const char *cmd, const char
}
if (p) {

•  command_found:
    shell = p;
  

•   while (*p) {
  

•       if ((unsigned char)*p == '/')
  

•           *p = '\\';
  

•       p = CharNext(p);
  

•   }
  

•   translate_char(p, '/', '\\');
  

  }

• RUBY_CRITICAL({

•   fRet = CreateProcess(shell, (char *)cmd, psa, psa,
  

•                        psa->bInheritHandle, dwCreationFlags, NULL, NULL,
  

•                        &aStartupInfo, &aProcessInformation);
  

•   errno = map_errno(GetLastError());
  

• });
• if (!fRet) {

•   child->pid = 0;         /* release the slot */
  

•   return NULL;
  

• }

• return child_result(CreateChild(cmd, shell, NULL, NULL, NULL, NULL), mode);
  +}

• CloseHandle(aProcessInformation.hThread);
  +rb_pid_t
  +rb_w32_aspawn(int mode, const char *prog, char *const *argv)
  +{

• int len, differ = 0, c_switch = 0;
• BOOL ntcmd = FALSE, tmpnt;
• const char *shell;
• char *cmd;

• child->hProcess = aProcessInformation.hProcess;
• child->pid = (rb_pid_t)aProcessInformation.dwProcessId;

• if (check_spawn_mode(mode)) return -1;

• if (!IsWinNT()) {

•   /* On Win9x, make pid positive similarly to cygwin and perl */
  

•   child->pid = -child->pid;
  

• if (!prog) prog = argv[0];
• if ((shell = getenv("COMSPEC")) &&

•   internal_cmd_match(prog, tmpnt = !is_command_com(shell))) {
  

•   ntcmd = tmpnt;
  

•   prog = shell;
  

•   c_switch = 1;
  

•   differ = 1;
  

• }
• else if ((cmd = dln_find_exe(prog, NULL))) {

•   if (cmd == prog) {
  

•       cmd = ALLOCA_N(char, strlen(cmd));
  

•       strcpy(cmd, prog);
  

•   }
  

•   translate_char(cmd, '/', '\\');
  

•   prog = cmd;
  

•   argv++;
  

•   differ = 1;
  

• }
• else if (strchr(prog, '/')) {

•   cmd = ALLOCA_N(char, strlen(cmd));
  

•   strcpy(cmd, prog);
  

•   translate_char(cmd, '/', '\\');
  

•   argv++;
  

•   differ = 1;
  

• }
• if (differ) {

•   char *progs[2];
  

•   int cmdlen;
  

•   progs[0] = (char *)prog;
  

•   progs[1] = NULL;
  

•   len = argv_size(progs, ntcmd);
  

•   cmdlen = len - 1;
  

•   if (c_switch) len += 3;
  

•   if (argv[0]) len += argv_size(argv, ntcmd);
  

•   cmd = ALLOCA_N(char, len);
  

•   join_argv(cmd, progs, ntcmd);
  

•   if (c_switch) {
  

•       memcpy(cmd + cmdlen, " /c", 4);
  

•       cmdlen += 3;
  

•   }
  

•   if (argv[0]) {
  

•       cmd[cmdlen++] = ' ';
  

•       join_argv(cmd + cmdlen, argv, ntcmd);
  

•   }
  

  }

• return child;

• else {

•   len = argv_size(argv, FALSE);
  

•   cmd = ALLOCA_N(char, len);
  

•   join_argv(cmd, argv, FALSE);
  

• }
• return child_result(CreateChild(cmd, prog, NULL, NULL, NULL, NULL), mode);
  }

@@ -1139,10 +1224,4 @@ insert(const char *path, VALUE vinfo)
}

-#ifdef HAVE_SYS_PARAM_H
-# include <sys/param.h>
-#else
-# define MAXPATHLEN 512
-#endif

static NtCmdLineElement **
@@ -1157,5 +1236,5 @@ cmdglob(NtCmdLineElement *patt, NtCmdLin
if (!(buf = malloc(patt->len + 1))) return 0;

• strncpy (buf, patt->str, patt->len);

• memcpy(buf, patt->str, patt->len + 1);
  buf[patt->len] = '\0';
  for (p = buf; *p; p = CharNext(p))
  @@ -1178,5 +1257,5 @@ cmdglob(NtCmdLineElement *patt, NtCmdLin
  //

-static bool
+static int
has_redirection(const char *cmd)
{
@@ -1208,4 +1287,10 @@ has_redirection(const char *cmd)
break;

•     case '%':
  

•       if (*++ptr != '_' && !ISALPHA(*ptr)) break;
  

•       while (*++ptr == '_' || ISALNUM(*ptr));
  

•       if (*ptr++ == '%') return TRUE;
  

•       break;
  

•     case '\\':
        ptr++;
  

Index: win32/win32.h

--- win32/win32.h (revision 21607)
+++ win32/win32.h (working copy)
@@ -139,5 +139,5 @@ extern DWORD rb_w32_osid(void);
#define stat(path,st) rb_w32_stat(path,st)
#undef execv
-#define execv(path,argv) do_aspawn(P_OVERLAY,path,argv)
+#define execv(path,argv) rb_w32_aspawn(P_OVERLAY,path,argv)
#if !defined(BORLANDC) && !defined(_WIN32_WCE)
#undef isatty
@@ -161,5 +161,5 @@ struct timezone {
extern void NtInitialize(int *, char ***);
extern int rb_w32_cmdvector(const char *, char ***);
-extern rb_pid_t pipe_exec(const char *, int, FILE **, FILE **);
+extern rb_pid_t rb_w32_pipe_exec(const char *, int, FILE **, FILE **);
extern int flock(int fd, int oper);
extern int rb_w32_accept(int, struct sockaddr *, int *);
@@ -207,6 +207,6 @@ extern int link(char *, char *);
extern int gettimeofday(struct timeval *, struct timezone *);
extern rb_pid_t waitpid (rb_pid_t, int *, int);
-extern int do_spawn(int, const char *);
-extern int do_aspawn(int, const char *, char **);
+extern rb_pid_t rb_w32_spawn(int, const char *, const char *prog);
+extern rb_pid_t rb_w32_aspawn(int, const char *, char *const *);
extern int kill(int, int);
extern int fcntl(int, int, ...);

--
Nobu Nakada

=end

[quix (James M. Lawrence)]

=begin
Nobu wrote:

Note there are other popular Windows shells such the
PowerShell, so the general strategy of checking for built-in
shell commands cannot work.

Is COMSPEC the PowerShell by the default on such systems?

Probably not, but that is not relevant to the point I wanted to make.

If system("built-in-shell-command", "1") fails on Unix, then it should
fail on Windows.

If it fails with an alternate shell on Windows, then it should fail
with all shells.

Why does ruby support system("built-in-shell-command", "1") for this
one case of Windows + cmd.exe?

=end

[nobu (Nobuyoshi Nakada)]

=begin
Hi,

At Tue, 20 Jan 2009 08:44:50 +0900,
James M. Lawrence wrote in [ruby-core:21449]:

Note there are other popular Windows shells such the
PowerShell, so the general strategy of checking for built-in
shell commands cannot work.

Is COMSPEC the PowerShell by the default on such systems?

Probably not, but that is not relevant to the point I wanted to make.

If system("built-in-shell-command", "1") fails on Unix, then it should
fail on Windows.

Many of built-in shell commands of Unix also exist as an
independent file. e.g., /bin/echo, /bin/test, /bin/[ and so on,
but not on Windows.

--
Nobu Nakada

=end

[quix (James M. Lawrence)]

=begin
Nobu wrote:

If system("built-in-shell-command", "1") fails on Unix, then it
should fail on Windows.

If it fails with an alternate shell on Windows, then it should fail
with all shells.

Why does ruby support system("built-in-shell-command", "1") for
this one case of Windows + cmd.exe?

Many of built-in shell commands of Unix also exist as an independent
file. e.g., /bin/echo, /bin/test, /bin/[ and so on, but not on
Windows.

So the reason is Unix emulation on mingw32/mswin32?

For those who want it, Unix emulation is already available with Cygwin
(which installs echo.exe, date.exe, etc). Those using mingw32/mswin32
do not expect a Unix-like system().

If I understand correctly, the purpose of this "magical" multi-arg
form of system() is to handle commands which both (1) exist in cmd.exe
and (2) exist as executables on a Unix machine. The only ones I see
fitting this description are echo, date, mkdir, rmdir, time, type, md.

Notice 'type' and 'md' have different meanings on Windows and Unix.
These will silently produce bugs when carried between Unix and Windows
machines, thanks to this magical system().

It is not only the Unix namesakes echo, date, etc. which are treated
specially, but all cmd.exe commands. The behavior is inconsistent
with all other platforms, plus Windows itself with an alternate shell.

I believe this "feature" is a mistake, but ultimately it is a minor
issue. Since it was the only thing which failed my system() spec, I
had to ask.

=end

[nobu (Nobuyoshi Nakada)]

=begin
Hi,

At Wed, 21 Jan 2009 10:25:09 +0900,
James M. Lawrence wrote in [ruby-core:21476]:

Many of built-in shell commands of Unix also exist as an independent
file. e.g., /bin/echo, /bin/test, /bin/[ and so on, but not on
Windows.

So the reason is Unix emulation on mingw32/mswin32?

No, of course. Such basic commands are built in the shell on
Windows, just I meant.

--
Nobu Nakada

=end

[quix (James M. Lawrence)]

=begin
Nobu wrote:

Many of built-in shell commands of Unix also exist as an
independent file. e.g., /bin/echo, /bin/test, /bin/[ and so on,
but not on Windows.

So the reason is Unix emulation on mingw32/mswin32?

No, of course. Such basic commands are built in the shell on
Windows, just I meant.

If you were not answering my question, then what is the answer? Why
does ruby support system("built-in-shell-command", "1") for this one
case of Windows + cmd.exe?

=end

[nobu (Nobuyoshi Nakada)]

=begin
Hi,

At Thu, 29 Jan 2009 10:49:14 +0900,
James M. Lawrence wrote in [ruby-core:21650]:

Many of built-in shell commands of Unix also exist as an
independent file. e.g., /bin/echo, /bin/test, /bin/[ and so on,
but not on Windows.

So the reason is Unix emulation on mingw32/mswin32?

No, of course. Such basic commands are built in the shell on
Windows, just I meant.

If you were not answering my question, then what is the answer? Why
does ruby support system("built-in-shell-command", "1") for this one
case of Windows + cmd.exe?

Just followed the habits, e.g., nmake.exe also can run built-in
commands such as echo, dir and so on.

--
Nobu Nakada

=end

[rogerdpack (Roger Pack)]

=begin
backport to 1.8.x would be nice. I'm not sure if you can currently pass parameters to batch files even :)
=end