Subject: bug#23173: closed (Re: [PATCH] Port redirect-debugging-output to non-GNU/Linux)

From: [email-masked] (GNU bug Tracking System)
Date: Wed, 06 Apr 2016 00:31:02 +0000

Your bug report

#23173: [PATCH] Port redirect-debugging-output to non-GNU/Linux

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
23173: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23173
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: [PATCH] Port redirect-debugging-output to non-GNU/Linux
From: Paul Eggert <[email-masked]>
To: [email-masked]
Date: Tue, 5 Apr 2016 17:30:32 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Closing this bug report, as the patch has been installed in master.

http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=6bccb19c9bef1189c8e853ff7cc16b889a3a57e3

http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=0322457e2bec0b9409a03887a8235dbe14e357f4

Subject: [PATCH] Port redirect-debugging-output to non-GNU/Linux
From: Paul Eggert <[email-masked]>
To: [email-masked]
Cc: Paul Eggert <[email-masked]>
Date: Thu, 31 Mar 2016 13:25:34 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Problem reported by Kylie McClain for musl in:
http://lists.gnu.org/archive/html/emacs-devel/2016-03/msg01592.html
* etc/DEBUG, etc/NEWS: Mention this.
* src/callproc.c (child_setup) [!MSDOS]:
* src/dispnew.c (init_display):
* src/emacs.c (main, Fdaemon_initialized):
* src/minibuf.c (read_minibuf_noninteractive):
* src/regex.c (xmalloc, xrealloc):
Prefer symbolic names like STDERR_FILENO to magic numbers like 2,
to make file-descriptor manipulation easier to follow.
* src/emacs.c (relocate_fd) [!WINDOWSNT]: Remove; no longer needed
now that we make sure stdin, stdout and stderr are open.  All uses
removed.
(main): Make sure standard FDs are OK.  Prefer symbolic names like
EXIT_FAILURE to magic numbers like 1.  Use bool for boolean.
* src/lisp.h (init_standard_fds): New decl.
* src/print.c (WITH_REDIRECT_DEBUGGING_OUTPUT) [GNU_LINUX]:
Remove; no longer needed.
(Fredirect_debugging_output): Define on all platforms, not just
GNU/Linux.  Redirect file descriptor, not stream, so that the code
works even if stderr is not an lvalue.  Report an error if the
file arg is neither a string nor nil.
(syms_of_print): Always define redirect-debugging-output.
* src/sysdep.c (force_open, init_standard_fds): New functions.
---
 etc/DEBUG      |  8 ++++----
 etc/NEWS       |  3 +++
 src/callproc.c | 58 +++-------------------------------------------------------
 src/dispnew.c  |  2 +-
 src/emacs.c    | 28 +++++++++++++++-------------
 src/lisp.h     |  1 +
 src/minibuf.c  | 12 ++++++------
 src/print.c    | 51 ++++++++++++++++++++++++---------------------------
 src/regex.c    |  4 ++--
 src/sysdep.c   | 29 +++++++++++++++++++++++++++++
 10 files changed, 88 insertions(+), 108 deletions(-)

diff --git a/etc/DEBUG b/etc/DEBUG
index eef67da..d5d5829 100644
--- a/etc/DEBUG
+++ b/etc/DEBUG
@@ -144,8 +144,8 @@ These are displayed as integer values (or structures, if you used the
 "--enable-check-lisp-object-type" option at configure time) that are
 hard to interpret, especially if they represent long lists.  You can
 use the 'pp' command to display them in their Lisp form.  That command
-displays its output on the standard error stream (on GNU/Linux, you
-can redirect that to a file using "M-x redirect-debugging-output").
+displays its output on the standard error stream, which you
+can redirect to a file using "M-x redirect-debugging-output".
 This means that if you attach GDB to a running Emacs that was invoked
 from a desktop icon, chances are you will not see the output at all,
 or it will wind up in an obscure place (check the documentation of
@@ -250,8 +250,8 @@ To see the current value of a Lisp Variable, use 'pv variable'.
 These commands send their output to stderr; if that is closed or
 redirected to some file you don't know, you won't see their output.
 This is particularly so for Emacs invoked on MS-Windows from the
-desktop shortcut.  On GNU/Linux, you can use the command
-'redirect-debugging-output' to redirect stderr to a file.
+desktop shortcut.  You can use the command 'redirect-debugging-output'
+to redirect stderr to a file.
 
 Note: It is not a good idea to try 'pr', 'pp', or 'pv' if you know that Emacs
 is in deep trouble: its stack smashed (e.g., if it encountered SIGSEGV
diff --git a/etc/NEWS b/etc/NEWS
index 726b4b9..e4d8a62 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -208,6 +208,9 @@ permanent and documented, and may be used by Lisp programs.  Its value
 is a list of currently open parenthesis positions, starting with the
 outermost parenthesis.
 
+** The function 'redirect-debugging-output' now works on platforms
+other than GNU/Linux.
+
 
 * Changes in Emacs 25.2 on Non-Free Operating Systems
 
diff --git a/src/callproc.c b/src/callproc.c
index db602f5..8578556 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1078,10 +1078,6 @@ usage: (call-process-region START END PROGRAM &optional DELETE BUFFER DISPLAY &r
   return unbind_to (count, val);
 }
 
-#ifndef WINDOWSNT
-static int relocate_fd (int fd, int minfd);
-#endif
-
 static char **
 add_env (char **env, char **new_env, char *string)
 {
@@ -1310,37 +1306,14 @@ child_setup (int in, int out, int err, char **new_argv, bool set_pgrp,
   return cpid;
 
 #else  /* not WINDOWSNT */
-  /* Make sure that in, out, and err are not actually already in
-     descriptors zero, one, or two; this could happen if Emacs is
-     started with its standard in, out, or error closed, as might
-     happen under X.  */
-  {
-    int oin = in, oout = out;
-
-    /* We have to avoid relocating the same descriptor twice!  */
-
-    in = relocate_fd (in, 3);
-
-    if (out == oin)
-      out = in;
-    else
-      out = relocate_fd (out, 3);
-
-    if (err == oin)
-      err = in;
-    else if (err == oout)
-      err = out;
-    else
-      err = relocate_fd (err, 3);
-  }
 
 #ifndef MSDOS
   /* Redirect file descriptors and clear the close-on-exec flag on the
      redirected ones.  IN, OUT, and ERR are close-on-exec so they
      need not be closed explicitly.  */
-  dup2 (in, 0);
-  dup2 (out, 1);
-  dup2 (err, 2);
+  dup2 (in, STDIN_FILENO);
+  dup2 (out, STDOUT_FILENO);
+  dup2 (err, STDERR_FILENO);
 
   setpgid (0, 0);
   tcsetpgrp (0, pid);
@@ -1359,31 +1332,6 @@ child_setup (int in, int out, int err, char **new_argv, bool set_pgrp,
 #endif  /* not WINDOWSNT */
 }
 
-#ifndef WINDOWSNT
-/* Move the file descriptor FD so that its number is not less than MINFD.
-   If the file descriptor is moved at all, the original is closed on MSDOS,
-   but not elsewhere as the caller will close it anyway.  */
-static int
-relocate_fd (int fd, int minfd)
-{
-  if (fd >= minfd)
-    return fd;
-  else
-    {
-      int new = fcntl (fd, F_DUPFD_CLOEXEC, minfd);
-      if (new == -1)
-	{
-	  emacs_perror ("while setting up child");
-	  _exit (EXIT_CANCELED);
-	}
-#ifdef MSDOS
-      emacs_close (fd);
-#endif
-      return new;
-    }
-}
-#endif /* not WINDOWSNT */
-
 static bool
 getenv_internal_1 (const char *var, ptrdiff_t varlen, char **value,
 		   ptrdiff_t *valuelen, Lisp_Object env)
diff --git a/src/dispnew.c b/src/dispnew.c
index 3a0532a..51caa5b 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -6038,7 +6038,7 @@ init_display (void)
 #endif
 
   /* If no window system has been specified, try to use the terminal.  */
-  if (! isatty (0))
+  if (! isatty (STDIN_FILENO))
     fatal ("standard input is not a tty");
 
 #ifdef WINDOWSNT
diff --git a/src/emacs.c b/src/emacs.c
index 95d1905..c21c9e3 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -721,6 +721,7 @@ main (int argc, char **argv)
     unexec_init_emacs_zone ();
 #endif
 
+  init_standard_fds ();
   atexit (close_output_streams);
 
 #ifdef HAVE_MODULES
@@ -899,24 +900,25 @@ main (int argc, char **argv)
       char *term;
       if (argmatch (argv, argc, "-t", "--terminal", 4, &term, &skip_args))
 	{
-	  int result;
-	  emacs_close (0);
-	  emacs_close (1);
-	  result = emacs_open (term, O_RDWR, 0);
-	  if (result < 0 || fcntl (0, F_DUPFD_CLOEXEC, 1) < 0)
+	  emacs_close (STDIN_FILENO);
+	  emacs_close (STDOUT_FILENO);
+	  int result = emacs_open (term, O_RDWR, 0);
+	  if (result != STDIN_FILENO
+	      || (fcntl (STDIN_FILENO, F_DUPFD_CLOEXEC, STDOUT_FILENO)
+		  != STDOUT_FILENO))
 	    {
 	      char *errstring = strerror (errno);
 	      fprintf (stderr, "%s: %s: %s\n", argv[0], term, errstring);
-	      exit (1);
+	      exit (EXIT_FAILURE);
 	    }
-	  if (! isatty (0))
+	  if (! isatty (STDIN_FILENO))
 	    {
 	      fprintf (stderr, "%s: %s: not a tty\n", argv[0], term);
-	      exit (1);
+	      exit (EXIT_FAILURE);
 	    }
 	  fprintf (stderr, "Using %s\n", term);
 #ifdef HAVE_WINDOW_SYSTEM
-	  inhibit_window_system = 1; /* -t => -nw */
+	  inhibit_window_system = true; /* -t => -nw */
 #endif
 	}
       else
@@ -1209,7 +1211,7 @@ Using an Emacs configured with --with-x-toolkit=lucid does not have this problem
       /* Started from GUI? */
       /* FIXME: Do the right thing if getenv returns NULL, or if
          chdir fails.  */
-      if (! inhibit_window_system && ! isatty (0) && ! ch_to_dir)
+      if (! inhibit_window_system && ! isatty (STDIN_FILENO) && ! ch_to_dir)
         chdir (getenv ("HOME"));
       if (skip_args < argc)
         {
@@ -2357,9 +2359,9 @@ from the parent process and its tty file descriptors.  */)
   /* Get rid of stdin, stdout and stderr.  */
   nfd = emacs_open ("/dev/null", O_RDWR, 0);
   err |= nfd < 0;
-  err |= dup2 (nfd, 0) < 0;
-  err |= dup2 (nfd, 1) < 0;
-  err |= dup2 (nfd, 2) < 0;
+  err |= dup2 (nfd, STDIN_FILENO) < 0;
+  err |= dup2 (nfd, STDOUT_FILENO) < 0;
+  err |= dup2 (nfd, STDERR_FILENO) < 0;
   err |= emacs_close (nfd) != 0;
 
   /* Closing the pipe will notify the parent that it can exit.
diff --git a/src/lisp.h b/src/lisp.h
index 7c8b452..65335fb 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4248,6 +4248,7 @@ struct tty_display_info;
 struct terminal;
 
 /* Defined in sysdep.c.  */
+extern void init_standard_fds (void);
 extern char *emacs_get_current_dir_name (void);
 extern void stuff_char (char c);
 extern void init_foreground_group (void);
diff --git a/src/minibuf.c b/src/minibuf.c
index 238a04a..41814c2 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -194,7 +194,7 @@ read_minibuf_noninteractive (Lisp_Object map, Lisp_Object initial,
   int c;
   unsigned char hide_char = 0;
   struct emacs_tty etty;
-  bool etty_valid;
+  bool etty_valid IF_LINT (= false);
 
   /* Check, whether we need to suppress echoing.  */
   if (CHARACTERP (Vread_hide_char))
@@ -203,10 +203,10 @@ read_minibuf_noninteractive (Lisp_Object map, Lisp_Object initial,
   /* Manipulate tty.  */
   if (hide_char)
     {
-      etty_valid = emacs_get_tty (fileno (stdin), &etty) == 0;
+      etty_valid = emacs_get_tty (STDIN_FILENO, &etty) == 0;
       if (etty_valid)
-	set_binary_mode (fileno (stdin), O_BINARY);
-      suppress_echo_on_tty (fileno (stdin));
+	set_binary_mode (STDIN_FILENO, O_BINARY);
+      suppress_echo_on_tty (STDIN_FILENO);
     }
 
   fwrite (SDATA (prompt), 1, SBYTES (prompt), stdout);
@@ -240,8 +240,8 @@ read_minibuf_noninteractive (Lisp_Object map, Lisp_Object initial,
       fprintf (stdout, "\n");
       if (etty_valid)
 	{
-	  emacs_set_tty (fileno (stdin), &etty, 0);
-	  set_binary_mode (fileno (stdin), O_TEXT);
+	  emacs_set_tty (STDIN_FILENO, &etty, 0);
+	  set_binary_mode (STDIN_FILENO, O_TEXT);
 	}
     }
 
diff --git a/src/print.c b/src/print.c
index 2b53d75..db2918f 100644
--- a/src/print.c
+++ b/src/print.c
@@ -775,15 +775,6 @@ debug_output_compilation_hack (bool x)
   print_output_debug_flag = x;
 }
 
-#if defined (GNU_LINUX)
-
-/* This functionality is not vitally important in general, so we rely on
-   non-portable ability to use stderr as lvalue.  */
-
-#define WITH_REDIRECT_DEBUGGING_OUTPUT 1
-
-static FILE *initial_stderr_stream = NULL;
-
 DEFUN ("redirect-debugging-output", Fredirect_debugging_output, Sredirect_debugging_output,
        1, 2,
        "FDebug output file: \nP",
@@ -793,30 +784,38 @@ Optional arg APPEND non-nil (interactively, with prefix arg) means
 append to existing target file.  */)
   (Lisp_Object file, Lisp_Object append)
 {
-  if (initial_stderr_stream != NULL)
-    {
-      block_input ();
-      fclose (stderr);
-      unblock_input ();
-    }
-  stderr = initial_stderr_stream;
-  initial_stderr_stream = NULL;
+  /* If equal to STDERR_FILENO, stderr has not been duplicated and is OK as-is.
+     Otherwise, this is a close-on-exec duplicate of the original stderr. */
+  static int stderr_dup = STDERR_FILENO;
+  int fd = stderr_dup;
 
-  if (STRINGP (file))
+  if (! NILP (file))
     {
       file = Fexpand_file_name (file, Qnil);
-      initial_stderr_stream = stderr;
-      stderr = emacs_fopen (SSDATA (file), NILP (append) ? "w" : "a");
-      if (stderr == NULL)
+
+      if (stderr_dup == STDERR_FILENO)
 	{
-	  stderr = initial_stderr_stream;
-	  initial_stderr_stream = NULL;
-	  report_file_error ("Cannot open debugging output stream", file);
+	  int n = fcntl (STDERR_FILENO, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
+	  if (n < 0)
+	    report_file_error ("dup", file);
+	  stderr_dup = n;
 	}
+
+      fd = emacs_open (SSDATA (ENCODE_FILE (file)),
+		       (O_WRONLY | O_CREAT
+			| (! NILP (append) ? O_APPEND : O_TRUNC)),
+		       0666);
+      if (fd < 0)
+	report_file_error ("Cannot open debugging output stream", file);
     }
+
+  fflush (stderr);
+  if (dup2 (fd, STDERR_FILENO) < 0)
+    report_file_error ("dup2", file);
+  if (fd != stderr_dup)
+    emacs_close (fd);
   return Qnil;
 }
-#endif /* GNU_LINUX */

 /* This is the interface for debugging printing.  */
@@ -2305,9 +2304,7 @@ priorities.  */);
   defsubr (&Sprint);
   defsubr (&Sterpri);
   defsubr (&Swrite_char);
-#ifdef WITH_REDIRECT_DEBUGGING_OUTPUT
   defsubr (&Sredirect_debugging_output);
-#endif
 
   DEFSYM (Qprint_escape_newlines, "print-escape-newlines");
   DEFSYM (Qprint_escape_multibyte, "print-escape-multibyte");
diff --git a/src/regex.c b/src/regex.c
index d5c58ae..af37936 100644
--- a/src/regex.c
+++ b/src/regex.c
@@ -215,7 +215,7 @@ xmalloc (size_t size)
   void *val = malloc (size);
   if (!val && size)
     {
-      write (2, "virtual memory exhausted\n", 25);
+      write (STDERR_FILENO, "virtual memory exhausted\n", 25);
       exit (1);
     }
   return val;
@@ -233,7 +233,7 @@ xrealloc (void *block, size_t size)
     val = realloc (block, size);
   if (!val && size)
     {
-      write (2, "virtual memory exhausted\n", 25);
+      write (STDERR_FILENO, "virtual memory exhausted\n", 25);
       exit (1);
     }
   return val;
diff --git a/src/sysdep.c b/src/sysdep.c
index 6154c13..67c9bd9 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -130,6 +130,35 @@ static const int baud_convert[] =
     1800, 2400, 4800, 9600, 19200, 38400
   };
 
+/* If FD is not already open, arrange for it to be open with FLAGS.  */
+static void
+force_open (int fd, int flags)
+{
+  if (dup2 (fd, fd) < 0 && errno == EBADF)
+    {
+      int n = open (NULL_DEVICE, flags);
+      if (n < 0 || (fd != n && (dup2 (n, fd) < 0 || emacs_close (n) != 0)))
+	{
+	  emacs_perror (NULL_DEVICE);
+	  exit (EXIT_FAILURE);
+	}
+    }
+}
+
+/* Make sure stdin, stdout, and stderr are open to something, so that
+   their file descriptors are not hijacked by later system calls.  */
+void
+init_standard_fds (void)
+{
+  /* Open stdin for *writing*, and stdout and stderr for *reading*.
+     That way, any attempt to do normal I/O will result in an error,
+     just as if the files were closed, and the file descriptors will
+     not be reused by later opens.  */
+  force_open (STDIN_FILENO, O_WRONLY);
+  force_open (STDOUT_FILENO, O_RDONLY);
+  force_open (STDERR_FILENO, O_RDONLY);
+}
+
 /* Return the current working directory.  The result should be freed
    with 'free'.  Return NULL on errors.  */
 char *
-- 
2.5.5