bug#13054: Assume POSIX 1003.1-1988 or later for execve.

bug#13054: Assume POSIX 1003.1-1988 or later for execve.

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 176 bytes --]

Attached is a patch to simplify Emacs that I discovered while looking
into the putenv/setenv problem.  It affects the NT port so I'm
CC'ing this to Eli to give him a heads-up.

[-- Attachment #2: execve.txt --]
[-- Type: text/plain, Size: 5235 bytes --]

=== modified file 'lib-src/ChangeLog'
--- lib-src/ChangeLog	2012-12-02 01:47:56 +0000
+++ lib-src/ChangeLog	2012-12-02 04:32:26 +0000
@@ -1,3 +1,8 @@
+2012-12-02  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Assume POSIX 1003.1-1988 or later for execve.
+	* ntlib.h (execve): New macro.
+
 2012-12-02  Kevin Ryde  <user42@zip.com.au>
 
 	* etags.c (Lisp_functions): Skip (defvar foo) declarations unless

=== modified file 'lib-src/ntlib.h'
--- lib-src/ntlib.h	2012-01-19 07:21:25 +0000
+++ lib-src/ntlib.h	2012-12-02 04:32:26 +0000
@@ -79,6 +79,7 @@
 
 /* map to MSVC names */
 #define execlp    _execlp
+#define execve    _execve
 #define execvp    _execvp
 #define fdopen	  _fdopen
 #ifndef fileno
@@ -104,4 +105,3 @@
 #undef _WINSOCK_H
 
 /* end of ntlib.h */
-

=== modified file 'nt/ChangeLog'
--- nt/ChangeLog	2012-12-01 20:09:30 +0000
+++ nt/ChangeLog	2012-12-02 04:32:26 +0000
@@ -1,3 +1,8 @@
+2012-12-02  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Assume POSIX 1003.1-1988 or later for execve.
+	* inc/ms-w32.h (execve): New macro.
+
 2012-12-01  Juanma Barranquero  <lekktu@gmail.com>
 
 	* config.nt: Sync with autogen/config.in.

=== modified file 'nt/inc/ms-w32.h'
--- nt/inc/ms-w32.h	2012-11-27 03:10:32 +0000
+++ nt/inc/ms-w32.h	2012-12-02 04:32:26 +0000
@@ -204,6 +204,7 @@
 
 /* Map to MSVC names.  */
 #define execlp    _execlp
+#define execve    _execve
 #define execvp    _execvp
 #define fdopen	  _fdopen
 #ifndef fileno

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2012-12-01 23:06:14 +0000
+++ src/ChangeLog	2012-12-02 04:32:26 +0000
@@ -1,3 +1,12 @@
+2012-12-02  Paul Eggert  <eggert@cs.ucla.edu>
+
+	Assume POSIX 1003.1-1988 or later for execve.
+	* callproc.c (Fcall_process):
+	* process.c (create_process):
+	Don't save and restore environ;	no longer needed.
+	* callproc.c (child_setup):
+	Use execve, not execvp, to preserve environ.
+
 2012-12-01  Paul Eggert  <eggert@cs.ucla.edu>
 
 	* xterm.c (x_draw_image_relief): Remove unused locals (Bug#10500).

=== modified file 'src/callproc.c'
--- src/callproc.c	2012-11-29 00:36:22 +0000
+++ src/callproc.c	2012-12-02 04:32:26 +0000
@@ -488,9 +488,6 @@
     }
 
   {
-    /* child_setup must clobber environ in systems with true vfork.
-       Protect it from permanent change.  */
-    char **save_environ = environ;
     int fd_error = fd1;
 
     if (fd_output >= 0)
@@ -594,7 +591,6 @@
       ptrdiff_t volatile count_volatile = count;
       ptrdiff_t volatile sa_count_volatile = sa_count;
       char **volatile new_argv_volatile = new_argv;
-      char **volatile new_save_environ = save_environ;
 
       pid = vfork ();
 
@@ -612,7 +608,6 @@
       count = count_volatile;
       sa_count = sa_count_volatile;
       new_argv = new_argv_volatile;
-      save_environ = new_save_environ;
     }
 
     if (pid == 0)
@@ -638,8 +633,6 @@
       emacs_close (fd_error);
 #endif /* not MSDOS */
 
-    environ = save_environ;
-
     /* Close most of our file descriptors, but not fd0
        since we will use that to read input from.  */
     emacs_close (filefd);
@@ -1092,10 +1085,6 @@
    Initialize inferior's priority, pgrp, connected dir and environment.
    then exec another program based on new_argv.
 
-   This function may change environ for the superior process.
-   Therefore, the superior process must save and restore the value
-   of environ around the vfork and the call to this function.
-
    If SET_PGRP, put the subprocess into a separate process group.
 
    CURRENT_DIR is an elisp string giving the path of the current
@@ -1298,11 +1287,7 @@
   setpgid (0, 0);
   tcsetpgrp (0, pid);
 
-  /* execvp does not accept an environment arg so the only way
-     to pass this environment is to set environ.  Our caller
-     is responsible for restoring the ambient value of environ.  */
-  environ = env;
-  execvp (new_argv[0], new_argv);
+  execve (new_argv[0], new_argv, env);
 
   emacs_write (1, "Can't exec program: ", 20);
   emacs_write (1, new_argv[0], strlen (new_argv[0]));

=== modified file 'src/process.c'
--- src/process.c	2012-11-27 05:17:07 +0000
+++ src/process.c	2012-12-02 04:32:26 +0000
@@ -1586,9 +1586,6 @@
   volatile int pty_flag = 0;
   volatile Lisp_Object lisp_pty_name = Qnil;
   volatile Lisp_Object encoded_current_dir;
-#if HAVE_WORKING_VFORK
-  char **volatile save_environ;
-#endif
 
   inchannel = outchannel = -1;
 
@@ -1688,12 +1685,6 @@
   pthread_sigmask (SIG_BLOCK, &blocked, 0);
 #endif
 
-#if HAVE_WORKING_VFORK
-  /* child_setup must clobber environ on systems with true vfork.
-     Protect it from permanent change.  */
-  save_environ = environ;
-#endif
-
 #ifndef WINDOWSNT
   pid = vfork ();
   if (pid == 0)
@@ -1819,10 +1810,6 @@
 
   /* Back in the parent process.  */
 
-#if HAVE_WORKING_VFORK
-  environ = save_environ;
-#endif
-
   XPROCESS (process)->pid = pid;
   if (0 <= pid)
     XPROCESS (process)->alive = 1;
@@ -1874,7 +1861,7 @@
       /* Wait for child_setup to complete in case that vfork is
 	 actually defined as fork.  The descriptor wait_child_setup[1]
 	 of a pipe is closed at the child side either by close-on-exec
-	 on successful execvp or the _exit call in child_setup.  */
+	 on successful execve or the _exit call in child_setup.  */
       {
 	char dummy;
 

bug#13054: Assume POSIX 1003.1-1988 or later for execve.

[Andreas Schwab]

Paul Eggert <eggert@cs.ucla.edu> writes:

> +	Assume POSIX 1003.1-1988 or later for execve.
> +	* callproc.c (Fcall_process):
> +	* process.c (create_process):
> +	Don't save and restore environ;	no longer needed.
> +	* callproc.c (child_setup):
> +	Use execve, not execvp, to preserve environ.

How does this have anything to do with POSIX?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#13054: Assume POSIX 1003.1-1988 or later for execve.

[Eli Zaretskii]

> Date: Sat, 01 Dec 2012 22:37:39 -0600
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: Eli Zaretskii <eliz@gnu.org>
> 
> Attached is a patch to simplify Emacs that I discovered while looking
> into the putenv/setenv problem.  It affects the NT port so I'm
> CC'ing this to Eli to give him a heads-up.

Thanks.  You can go ahead and commit this, these changes don't affect
MS-Windows at all (except in trivial "a = b; b = a;" way, without
changing a or b in between).

However, please omit these changes from the changeset:

> === modified file 'lib-src/ntlib.h'
> --- lib-src/ntlib.h	2012-01-19 07:21:25 +0000
> +++ lib-src/ntlib.h	2012-12-02 04:32:26 +0000
> @@ -79,6 +79,7 @@
>  
>  /* map to MSVC names */
>  #define execlp    _execlp
> +#define execve    _execve
>  #define execvp    _execvp
>  #define fdopen	  _fdopen
>  #ifndef fileno
> @@ -104,4 +105,3 @@
>  #undef _WINSOCK_H
>  
>  /* end of ntlib.h */
> -
> 
> === modified file 'nt/ChangeLog'
> --- nt/ChangeLog	2012-12-01 20:09:30 +0000
> +++ nt/ChangeLog	2012-12-02 04:32:26 +0000
> @@ -1,3 +1,8 @@
> +2012-12-02  Paul Eggert  <eggert@cs.ucla.edu>
> +
> +	Assume POSIX 1003.1-1988 or later for execve.
> +	* inc/ms-w32.h (execve): New macro.
> +
>  2012-12-01  Juanma Barranquero  <lekktu@gmail.com>
>  
>  	* config.nt: Sync with autogen/config.in.
> 
> === modified file 'nt/inc/ms-w32.h'
> --- nt/inc/ms-w32.h	2012-11-27 03:10:32 +0000
> +++ nt/inc/ms-w32.h	2012-12-02 04:32:26 +0000
> @@ -204,6 +204,7 @@
>  
>  /* Map to MSVC names.  */
>  #define execlp    _execlp
> +#define execve    _execve
>  #define execvp    _execvp
>  #define fdopen	  _fdopen
>  #ifndef fileno
> 

They are not needed.  (The Windows build of Emacs doesn't use any
functions of the exec* family, because they are fundamentally broken
on Windows.  I will look into removing the other exec* macros from
Windows-specific files.)

bug#13054: Assume POSIX 1003.1-1988 or later for execve.

[Paul Eggert]

On 12/02/2012 12:27 AM, Andreas Schwab wrote:
> How does this have anything to do with POSIX?

The patched version uses execve, the unpatched version doesn't.
Long ago, it made sense to avoid execve, because some pre-POSIX
platforms lacked execve.  (The FreeBSD man pages say that
execve was introduced in 4.2BSD, so I assume 4.1BSD lacked it.)
execve was standardized in POSIX 1003.1-1988, though.  It's
safe to assume execve now on all POSIXish platforms, and doing
so simplifies Emacs a bit.

bug#13054: Assume POSIX 1003.1-1988 or later for execve.

[Andreas Schwab]

Paul Eggert <eggert@cs.ucla.edu> writes:

> The patched version uses execve, the unpatched version doesn't.
> Long ago, it made sense to avoid execve, because some pre-POSIX
> platforms lacked execve.

execvp is just a wrapper around execve, so avoiding the latter doesn't
make sense.  There must be some other reason why execvp is used.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#13054: Assume POSIX 1003.1-1988 or later for execve.

[Paul Eggert]

On 12/02/2012 10:48 AM, Andreas Schwab wrote:
> execvp is just a wrapper around execve
> There must be some other reason why execvp is used.

Perhaps the author of that part of Emacs simply
didn't know about execve.  That would explain things.

It sounds like the main issue here is about mentioning
POSIX in the ChangeLog, so I removed that, along with
removing the other stuff Eli asked to remove, and
installed it as trunk bzr 111064.

bug#13054: Assume POSIX 1003.1-1988 or later for execve.

[Andreas Schwab]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Perhaps the author of that part of Emacs simply
> didn't know about execve.  That would explain things.

I think the original author just wanted child_setup to search PATH.  The
change logs don't give any indication whether that was ever necessary.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."