Re: open-process and related functions for MinGW Guile

[Eli Zaretskii]

> Date: Tue, 12 Aug 2014 22:44:03 +0300
> From: Eli Zaretskii <address@hidden>
> Cc: address@hidden, address@hidden
> 
> > From: Mark H Weaver <address@hidden>
> > Cc: address@hidden,  address@hidden
> > Date: Tue, 12 Aug 2014 14:08:48 -0400
> > 
> > > +/* The funny conditional avoids a compiler warning in status:stop_sig.  
> > > */
> > > +# define WIFSTOPPED(stat_val)  ((stat_val) == (stat_val) ? 0 : 0)
> > 
> > What is the warning?
> 
> That stat_val is unused (because the value is always zero).

The warning is:

  posix.c: In function 'scm_status_stop_sig':
  posix.c:832:7: warning: variable 'lstatus' set but not used 
[-Wunused-but-set-variable]

> > Would ((stat_val), 0) also fix it?
> 
> I'm not sure, but I will use it if it will.

This produces a different warning:

  posix.c: In function 'scm_status_stop_sig':
  posix.c:835:7: warning: left-hand operand of comma expression has no effect 
[-Wunused-value]

> > > +  for (i = 0; extensions[i]; i++)
> > > +    {
> > > +      abs_namelen = SearchPath (path, program, extensions[i],
> > > +                         MAX_PATH, abs_name, NULL);
> > > +      if (0 < abs_namelen && abs_namelen <= MAX_PATH)    /* found! */
> > > + break;
> > > +    }
> > 
> > This search is not done correctly.  If I ask to run program "foo", and
> > "foo.cmd" comes early in the path and "foo.exe" comes late in the path,
> > it should be "foo.cmd" that gets run, not "foo.exe".
> 
> No, the search order is exactly the one used by the Windows shell.

It looks like I've misread your comment: you meant that the search for
extensions should be inner to the search of directories on PATH.  I
agree; I show below the relevant portion of the patch after fixing
that.

> > How can we ensure that those strings are passed reliably and
> > losslessly, without relying on heuristics that only work some of the
> > time?
> 
> This is not heuristics, this is the official documented way to quote
> and escape quotes on the command line.  Working by those rules will
> not cause any losses.

The quoting rules are publicly documented here:

  http://msdn.microsoft.com/en-us/library/17w5ykft(v=vs.120).aspx

> > > +      /* cmd.exe needs a different style of quoting: all the arguments
> > > +  beyond the /c switch are enclosed in an extra pair of quotes,
> > > +  and not otherwise quoted/escaped. */
> > 
> > Again, I think this should be handled at a higher level in the stack.
> > It would be good to have primitives at the C level that don't include
> > such unreliable heuristics.
> 
> Sorry, I don't understand: what heuristics?  The way cmd.exe handles
> quotes is well documented, and the code implements those rules.
> There's no heuristics here, anywhere.

The way cmd.exe handles quoted command lines is documented here:

  
http://www.microsoft.com/resources/documentation/windows/xp/all/proddocs/en-us/cmd.mspx?mfr=true

(under "Remarks", see the "Processing quotation marks" bullet).

> > > +      /* Gnulib's 'pipe' opens the pipe in binary mode, but we don't
> > > +  want to read text-mode input of subprocesses in binary more,
> > > +  because then we will get the ^M (a.k.a. "CR") characters we
> > > +  don't expect.  */
> > > +      _setmode (c2p[0], _O_TEXT);
> > > +    }
> > 
> > We should do the newline conversion elsewhere in Guile's I/O stack, so
> > that it is possible to do binary I/O with subprocesses.
> 
> That's fine by me, but the default should still be text I/O.  Once
> there is a way to propagate the text/binary mode to this level, a
> condition can be added not to do the above.  IOW, this is something
> for future changes; for now, the vast majority of pipes need text-mode
> I/O, so leaving this at binary will break much more code than using
> text I/O.

According to this discussion on guile-user a year ago:

  http://lists.gnu.org/archive/html/guile-user/2013-06/msg00070.html
  http://lists.gnu.org/archive/html/guile-user/2013-06/msg00080.html

newline conversion for ports is not yet implemented.  Did something
change since then?  If not, then for now there's only one I/O mode
possible here, and that's got to be text mode.

Here's the search on PATH part of lookup_cmd after fixing the search
order:

+  path = getenv ("PATH");
+  if (!path)   /* shouldn't happen, really */
+    path = ".";
+  dir = sep = path = strdup (path);
+  for ( ; sep && *sep; dir = sep + 1)
+    {
+      int i;
+
+      sep = strpbrk (dir, ";");
+      if (sep == dir)  /* two or more ;'s in a row */
+       continue;
+      if (sep)
+       *sep = '\0';
+      for (i = 0; extensions[i]; i++)
+       {
+         abs_namelen = SearchPath (dir, program, extensions[i],
+                                   MAX_PATH, abs_name, NULL);
+         if (0 < abs_namelen && abs_namelen <= MAX_PATH)       /* found! */
+           break;
+       }
+      if (extensions[i])       /* found! */
+       break;
+      if (sep)
+       *sep = ';';
+    }
+
+  free (path);