Hello, Eli, Eli Zaretskii <e...@gnu.org> skribis:
> This is a sequel to the thread that started here: > > http://lists.gnu.org/archive/html/guile-devel/2014-02/msg00047.html > > As agreed with Mark at the end of that thread, please find below > patches that enable open-process and friends in the MinGW build of > Guile. The main changes since the patches I presented in February > are: > > . Guile's standard handles are not redirected before running the > child process. > > . The code which runs the child process supports both a Unixy shell > (if available), which is useful for the test suite, the stock > Windows shell cmd.exe, and other programs. This support includes > correct handling of quoted command-line arguments. > > . Translation of signals to exit status and back is based on a > mapping that produces signal values identical to the ones in > signal.h, as opposed to some convention private to Guile. > > . waitpid emulation supports WNOHANG (required to pass the > rnrs-libraries test). Nice! Preliminary comments below. > --- libguile/posix.c~ 2014-06-22 19:08:35.862625000 +0300 > +++ libguile/posix.c 2014-06-29 18:36:02.000000000 +0300 > @@ -84,6 +84,32 @@ > #if HAVE_SYS_WAIT_H > # include <sys/wait.h> > #endif > +#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. In addition... > +# 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. > -#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. > 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.) > - 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? > --- /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”, and add the LGPLv3+ license header. > +/* Translate abnormal exit status of Windows programs into the signal > + that terminated the program. This is required to support scm_kill > + and WTERMSIG. */ > + > +struct signal_and_status { > + int sig; > + DWORD status; > +}; > + > +static const struct signal_and_status sigtbl[] = { > + {SIGSEGV, 0xC0000005}, /* access to invalid address */ Opening braces on a line of their own; spaces around braces. Thank you, Ludo’.
