From: Andy Wingo <wingo@pobox.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: "Ludovic Courtès" <ludo@gnu.org>, guile-devel@gnu.org
Subject: Re: Support open-process and friends on MS-Windows
Date: Sat, 25 Jun 2016 16:43:34 +0200 [thread overview]
Message-ID: <87a8i9ba7d.fsf@pobox.com> (raw)
In-Reply-To: <83r3blz96w.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 25 Jun 2016 16:31:35 +0300")
On Sat 25 Jun 2016 15:31, Eli Zaretskii <eliz@gnu.org> writes:
> [Please disregard the previous patch and use this one instead.]
Thanks for the updates. I have a couple of additional nits, having
applied and built the patch.
> @@ -659,7 +663,7 @@ SCM_DEFINE (scm_kill, "kill", 2, 0, 0,
> #else
> /* Mingw has raise(), but not kill(). (Other raw DOS environments might
> be similar.) Use raise() when the requested pid is our own process,
> - otherwise bomb. */
> + otherwise TerminateProcess. */
> if (scm_to_int (pid) == getpid ())
> {
> if (raise (scm_to_int (sig)) != 0)
> @@ -673,6 +677,10 @@ SCM_DEFINE (scm_kill, "kill", 2, 0, 0,
> goto err;
> }
> }
> +#ifdef __MINGW32__
> + if (w32_kill_proc (scm_to_int (pid), scm_to_int (sig)) != 0)
> + SCM_SYSERROR;
> +#endif /* __MINGW32__ */
> #endif
> return SCM_UNSPECIFIED;
> }
Here we effectively have two mingw blocks. Can the previous one be
removed? If not, let's fold it into `w32_kill_proc', and in any case
let's rename `w32_kill_proc' to `kill', and then define HAVE_KILL.
> @@ -1141,7 +1143,7 @@ SCM_DEFINE (scm_execl, "execl", 1, 0, 1,
>
> exec_argv = scm_i_allocate_string_pointers (args);
>
> - execv (exec_file, exec_argv);
> + execv (exec_file, (char const * const *)exec_argv);
> SCM_SYSERROR;
>
> /* not reached. */
make[3]: Entering directory '/home/wingo/src/guile-2.0/libguile'
CC libguile_2.0_la-posix.lo
posix.c: In function 'scm_execl':
posix.c:1146:21: warning: passing argument 2 of 'execv' from incompatible pointer type [-Wincompatible-pointer-types]
execv (exec_file, (char const * const *)exec_argv);
^
In file included from ../lib/unistd.h:40:0,
from posix.c:50:
/home/wingo/.guix-profile/include/unistd.h:566:12: note: expected 'char * const*' but argument is of type 'const char * const*'
extern int execv (const char *__path, char *const __argv[])
^~~~~
What should be done here?
Similarly in the following cases.
> @@ -1408,16 +1433,16 @@ scm_open_process (SCM mode, SCM prog, SCM args)
> if (err > 0)
> {
> char *msg = strerror (errno);
> - fprintf (fdopen (err, "a"), "In execlp of %s: %s\n",
> + fprintf (fdopen (err, "a"), "In execvp of %s: %s\n",
> exec_file, msg);
> }
>
> _exit (EXIT_FAILURE);
> +#endif /* HAVE_FORK */
> /* Not reached. */
> return SCM_BOOL_F;
> }
> #undef FUNC_NAME
> -#endif /* HAVE_FORK */
This change updates to provide `open-process' effectively on all systems
instead of only if `fork' is available, but then it causes a runtime
error if the system doesn't have `fork' and isn't `mingw32'. I guess
that's OK in practice. Would you mind updating the documentation in
doc/ref/posix.texi to indicate this? Thanks.
Cheers, and thanks for dealing with the late feedback.
Andy
next prev parent reply other threads:[~2016-06-25 14:43 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-24 9:51 Support open-process and friends on MS-Windows Eli Zaretskii
2016-06-24 10:45 ` Andy Wingo
2016-06-24 13:20 ` Eli Zaretskii
2016-06-24 11:49 ` Ludovic Courtès
2016-06-24 13:25 ` Eli Zaretskii
2016-06-25 9:11 ` Eli Zaretskii
2016-06-25 9:51 ` Andy Wingo
2016-06-25 10:22 ` Eli Zaretskii
2016-06-25 13:02 ` Ludovic Courtès
2016-06-25 13:20 ` Eli Zaretskii
2016-06-25 13:31 ` Eli Zaretskii
2016-06-25 14:43 ` Andy Wingo [this message]
2016-06-25 15:01 ` Eli Zaretskii
2016-07-02 23:02 ` Mark H Weaver
2016-07-03 3:47 ` Eli Zaretskii
2016-07-03 17:36 ` Eli Zaretskii
2016-07-05 7:44 ` Mark H Weaver
2016-07-05 8:04 ` Ludovic Courtès
2016-07-05 15:56 ` Eli Zaretskii
2016-07-11 8:09 ` Ludovic Courtès
2016-07-11 14:49 ` Eli Zaretskii
2016-07-05 15:51 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/guile/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87a8i9ba7d.fsf@pobox.com \
--to=wingo@pobox.com \
--cc=eliz@gnu.org \
--cc=guile-devel@gnu.org \
--cc=ludo@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).