GNU bug report logs - #23173
[PATCH] Port redirect-debugging-output to non-GNU/Linux

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 23173 in the body.
You can then email your comments to 23173 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: bug-gnu-emacs <at> gnu.org
Cc: Paul Eggert <eggert <at> cs.ucla.edu>
Subject: [PATCH] Port redirect-debugging-output to non-GNU/Linux
Date: Thu, 31 Mar 2016 13:25:34 -0700

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

Message #10 received at 23173-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 23173-done <at> debbugs.gnu.org
Subject: Re: [PATCH] Port redirect-debugging-output to non-GNU/Linux
Date: Tue, 5 Apr 2016 17:30:32 -0700

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.