> From: l...@gnu.org (Ludovic Courtès)
> Cc: Mark H Weaver <m...@netris.org>, guile-devel@gnu.org
> Date: Sun, 29 Jun 2014 22:21:28 +0200
>
> > +#ifdef __MINGW32__
> > +
> > +#include <c-strcase.h>
> > +
> > +# define WEXITSTATUS(stat_val) ((stat_val) & 255)
> > +# define WIFEXITED(stat_val) (((stat_val) & 0xC0000000) == 0)
> > +# define WIFSIGNALED(stat_val) (((stat_val) & 0xC0000000) == 0xC0000000)
> > +# define WTERMSIG(stat_val) win32_status_to_termsig (stat_val)
> > +/* The funny conditional avoids a compiler warning in status:stop_sig. */
> > +# define WIFSTOPPED(stat_val) ((stat_val) == (stat_val) ? 0 : 0)
> > +# define WSTOPSIG(stat_var) (0)
>
> I think this was raised in the previous discussion: it looks a bit like
> black magic, so there should be a comment explaining why this is needed,
> how the constants were chosen, etc.
Most of the magic is gone in this version. I will add a comment about
0xC0000000.
> > +# include <process.h>
> > +# define HAVE_WAITPID 1
> > + static int win32_status_to_termsig (DWORD);
> > + static int win32_signal_to_status (int);
> > +# define getuid() (500) /* Local Administrator */
> > +# define getgid() (513) /* None */
> > +# define setuid(u) (0)
> > +# define setgid(g) (0)
> > +# define WIN32_LEAN_AND_MEAN
> > +# include <windows.h>
> > +# define WNOHANG 1
> > + int waitpid (intptr_t, int *, int);
> > +# include "win32-proc.c"
>
> ... what would you think of putting all this in a Gnulib module? It
> would benefit all GNU packages and probably get more testing.
Gnulib already has such a module, but its design and implementation is
based on wrong premises. We've been through that with Mark back in
February.
And my experience with Gnulib responsiveness hasn't changed much since
then: 2 tiny patches I submitted were accepted, but a larger patch to
nl_langinfo, which is very important for Guile, was left without a
comment for the past 3 weeks.
> > -#ifdef HAVE_SETEGID
> > SCM_DEFINE (scm_setegid, "setegid", 1, 0, 0,
> > (SCM id),
> > "Sets the effective group ID to the integer @var{id}, provided the
> > process\n"
>
> This should be a separate change, and it’s dubious since there could be
> platforms without setegid.
Which ones?
> > exec_argv = scm_i_allocate_string_pointers (args);
> >
> > - execv (exec_file, exec_argv);
> > + execv (exec_file, (char const * const *)exec_argv);
>
> This should be a separate change (if at all needed.)
It fixes a compiler warning.
> > - if (reading)
> > + if (reading)
> > {
> > close (c2p[1]);
> > - read_port = scm_fdes_to_port (c2p[0], "r0", sym_read_pipe);
> > + read_port = scm_fdes_to_port (c2p[0], "r", sym_read_pipe);
> > + scm_setvbuf (read_port, scm_from_int (_IONBF), SCM_UNDEFINED);
> > }
> > if (writing)
> > {
> > close (p2c[0]);
> > - write_port = scm_fdes_to_port (p2c[1], "w0", sym_write_pipe);
> > + write_port = scm_fdes_to_port (p2c[1], "w", sym_write_pipe);
> > + scm_setvbuf (write_port, scm_from_int (_IONBF), SCM_UNDEFINED);
>
> This reverts a43fa1b. Could you explain why it’s needed, and make it a
> separate patch?
Ignore this, I wasn't aware a change was made there.
> > --- /dev/null 1970-01-01 02:00:00 +0200
> > +++ libguile/win32-proc.c 2014-06-29 11:26:08 +0300
>
> Please call it “w32-proc.c” or “woe32-proc.c”
I was just following the example of win32-uname.c.
Thanks for the other feedback.
