> Date: Tue, 12 Aug 2014 22:44:03 +0300
> From: Eli Zaretskii <e...@gnu.org>
> Cc: l...@gnu.org, guile-devel@gnu.org
>
> > From: Mark H Weaver <m...@netris.org>
> > Cc: l...@gnu.org, guile-devel@gnu.org
> > 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);
