From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.lisp.guile.devel Subject: Re: [PATCH] Implement open-process and related functions on MinGW Date: Sat, 22 Feb 2014 18:35:25 +0200 Message-ID: <83lhx32jhu.fsf@gnu.org> References: <834n3x8o7m.fsf@gnu.org> <83y519788a.fsf@gnu.org> <871tz0d5vc.fsf@gnu.org> <83iosc76kz.fsf@gnu.org> <87vbwc72dp.fsf_-_@gnu.org> <8361o74e0k.fsf@gnu.org> <87zjljjg7r.fsf@yeeloong.lan> Reply-To: Eli Zaretskii NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE X-Trace: ger.gmane.org 1393086943 13826 80.91.229.3 (22 Feb 2014 16:35:43 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 22 Feb 2014 16:35:43 +0000 (UTC) Cc: ludo@gnu.org, guile-devel@gnu.org To: Mark H Weaver Original-X-From: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Sat Feb 22 17:35:47 2014 Return-path: Envelope-to: guile-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1WHFYI-0002DU-Qy for guile-devel@m.gmane.org; Sat, 22 Feb 2014 17:35:47 +0100 Original-Received: from localhost ([::1]:50109 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WHFYI-0001y1-DW for guile-devel@m.gmane.org; Sat, 22 Feb 2014 11:35:46 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:33632) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WHFY9-0001xr-5B for guile-devel@gnu.org; Sat, 22 Feb 2014 11:35:42 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WHFY2-0002ZD-IA for guile-devel@gnu.org; Sat, 22 Feb 2014 11:35:37 -0500 Original-Received: from mtaout26.012.net.il ([80.179.55.182]:43154) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WHFY1-0002Z2-W1; Sat, 22 Feb 2014 11:35:30 -0500 Original-Received: from conversion-daemon.mtaout26.012.net.il by mtaout26.012.net.il (HyperSendmail v2007.08) id <0N1E00800OJLRQ00@mtaout26.012.net.il>; Sat, 22 Feb 2014 18:33:41 +0200 (IST) Original-Received: from HOME-C4E4A596F7 ([87.69.4.28]) by mtaout26.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0N1E0014YOO5W470@mtaout26.012.net.il>; Sat, 22 Feb 2014 18:33:41 +0200 (IST) In-reply-to: <87zjljjg7r.fsf@yeeloong.lan> X-012-Sender: halo1@inter.net.il X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x X-Received-From: 80.179.55.182 X-BeenThere: guile-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Developers list for Guile, the GNU extensibility library" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Original-Sender: guile-devel-bounces+guile-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.lisp.guile.devel:16916 Archived-At: > From: Mark H Weaver > Cc: ludo@gnu.org (Ludovic Court=E8s), guile-devel@gnu.org > Date: Sat, 22 Feb 2014 10:54:16 -0500 >=20 > > diff --git a/libguile/posix.c b/libguile/posix.c > > index 0443f95..69652a3 100644 > > --- a/libguile/posix.c > > +++ b/libguile/posix.c > > @@ -85,6 +85,27 @@ > > #if HAVE_SYS_WAIT_H > > # include > > #endif > > +#ifdef __MINGW32__ > > +# define WEXITSTATUS(stat_val) ((stat_val) & 255) > > +/* Windows reports exit status from fatal exceptions as 0xC0000n= nn > > + codes, see winbase.h. We usurp codes above 0xC0000200 for SI= Gxxx > > + signals. */ > > +# define WIFEXITED(stat_val) (((stat_val) & 0xC0000000) =3D= =3D 0) > > +# define WIFSIGNALED(stat_val) (((stat_val) & 0xC0000000) !=3D 0= ) > > +# define WTERMSIG(stat_val) ((stat_val > 0xC0000200) ? stat_v= al - 0xC0000200 : stat_val) > > +# define WIFSTOPPED(stat_val) (0) > > +# define WSTOPSIG(stat_var) (0) >=20 > Definitions for these on MinGW are available in Gnulib's 'sys_wait' > module. I'll import it soon. Please don't: that gnulib module is dead wrong for Windows. If we us= e it, we will be unable to report even the simplest error exits, let alone programs that crashed due to a fatal signal. > > +# include > > +# define waitpid(p,s,o) _cwait(s, p, WAIT_CHILD) > > +# define HAVE_WAITPID 1 >=20 > Gnulib has a 'waitpid' module that implements it on MinGW. Can we = just > use that, and then assume it is there instead of setting HAVE_WAITP= ID? The gnulib waitpid module does exactly what I did above. If you are more comfortable with including it than with a single trivial macro, then go ahead. But in general, due to the known problems with gettin= g Windows-specific patches to gnulib approved and admitted, I'd advise to stay away of gnulib as much as possible, when Windows is concerned. E.g., what if tomorrow we would need to make the waitpid emulation more sophisticated? > I'll add it to my list of modules to import, along with the others > mentioned in this message. >=20 > > +# define getuid() (500) /* Local Administrator */ > > +# define getgid() (513) /* None */ > > +# define setuid(u) (0) > > +# define setgid(g) (0) >=20 > Hmm. I'm not sure about these. If we can't do better than this, I > think we should arrange to not bind these functions in MinGW, and n= ot > call them in our code. What do you think? Which functions are you talking about, specifically? In general, where the Windows equivalent is to do nothing, my advice would be to have a no-op implementation, rather than an unbound function. The former approach makes it much easier to write portable Guile scripts, instead of spreading boundness checks all over the place. FWIW, Emacs takes the former approach with very good results. > > +# define pipe(f) _pipe(f, 0, O_NOINHERIT) >=20 > Gnulib has a 'pipe' module that implements it on MinGW. Same response as for waitpid. > > +#ifdef __MINGW32__ > > + else > > + { > > + HANDLE ph =3D OpenProcess (PROCESS_TERMINATE, 0, scm_to_in= t (pid)); > > + int s =3D scm_to_int (sig); > > + > > + if (!ph) > > +=09{ > > +=09 errno =3D EPERM; > > +=09 goto err; > > +=09} > > + if (!TerminateProcess (ph, 0xC0000200 + s)) > > +=09{ > > +=09 errno =3D EINVAL; > > +=09 goto err; > > +=09} > > + CloseHandle (ph); > > + } > > +#endif=09/* __MINGW32__ */ > > #endif > > return SCM_UNSPECIFIED; > > } >=20 > This change is not mentioned in the commit log. Sorry, I sent a wrong commit log. Here's the correct one: =09* posix.c (WEXITSTATUS, WIFEXITED, WIFSIGNALED, WTERMSIG) =09(WIFSTOPPED, WSTOPSIG) [__MINGW32__]: Define for MinGW. =09(waitpid, getuid, getgid, setuid, setgid, pipe) [__MINGW32__]: =09Define replacements for MinGW. =09(scm_kill) [__MINGW32__]: Implement killing a process on =09MS-Windows. =09(scm_waitpid) [__MINGW32__]: Ignore the options argument. =09(scm_status_exit_val, scm_getuid): No longer ifdef'ed away for Min= GW. =09(scm_open_process) [__MINGW32__]: Implement starting a process =09on MS-Windows. Fix a typo in an error message (execlp instead =09of execvp). =09(scm_init_popen): No longer ifdef'ed away when HAVE_FORK is not =09defined. =09(scm_init_posix): The scm_init_popen feature is now available =09even if HAVE_FORK is not defined. > Can you use the GNU coding conventions for placement of brackets? Sorry, I don't follow: which part is not according to the GNU coding conventions? > What is the meaning of 0xC0000200? It would be good to add a comme= nt > explaining what that means, or better yet use preprocessor defines > (if they are available in a header). It's explained where WEXIT* macros are defined. I will add another comment here pointing there. > Ideally this should be implemented in Gnulib, but I'm okay with > including this in Guile for now. Thanks. As I wrote above, gnulib doesn't have a functional implementation of these macros and the related functionality. > > @@ -720,6 +760,7 @@ SCM_DEFINE (scm_waitpid, "waitpid", 1, 1, 0, > > { > > int i; > > int status; > > +#ifndef __MINGW32__ > > int ioptions; > > if (SCM_UNBNDP (options)) > > ioptions =3D 0; > > @@ -728,6 +769,7 @@ SCM_DEFINE (scm_waitpid, "waitpid", 1, 1, 0, > > /* Flags are interned in scm_init_posix. */ > > ioptions =3D scm_to_int (options); > > } > > +#endif > > SCM_SYSCALL (i =3D waitpid (scm_to_int (pid), &status, ioption= s)); > > if (i =3D=3D -1) > > SCM_SYSERROR; >=20 > Gnulib has a 'waitpid' module that apparently implements it on MinG= W. > Can we use that? See above. > Also, this change is not mentioned in the commit log. See above. > Thanks for working on this, but in a multithreaded program, it's no= good > to change the file descriptors in the main program temporarily befo= re > spawning, and then restore them afterwards. We'll have to find ano= ther > way of doing this. Threads don't work in the MinGW build anyway, and no one was able to help me understand why (see the discussions last August). As long as this limitation exists, this is a non-issue, and certainly better tha= n not having this functionality available at all. The only other way I know of is to bypass Posix-like functions like 'open', 'close', and 'dup', and go to the level of Win32 API, where a function that creates a child subprocess can accept handles for the child's standard I/O. If you will be comfortable with a lot more Widows specific stuff, I can work on that. Thanks.