RE: [PATCH 1/1] fport_print: handle ttyname ENODEV

From 94b338f144441483b0d0513be5c90d88c7aaeffb Mon Sep 17 00:00:00 2001
From: Rob Browning <[email protected]>
Date: Fri, 17 Jan 2025 11:45:26 -0600
Subject: [PATCH 1/1] fport_print: handle ttyname ENODEV

In some situations, ttyname may return ENODEV even though isatty is
true.  From ttyname(3):

  A process that keeps a file descriptor that refers to a pts(4) device
  open when switching to another mount namespace that uses a different
  /dev/ptmx instance may still accidentally find that a device path of
  the same name for that file descriptor exists.  However, this device
  path refers to a different device and thus can't be used to access the
  device that the file descriptor refers to.  Calling ttyname() or
  ttyname_r() on the file descriptor in the new mount namespace will
  cause these functions to return NULL and set errno to ENODEV.

Observed in a Debian riscv64 porterbox schroot.

When ttyname fails with ENODEV, just include the file descriptor integer
value instead.  Call ttyname() rather than scm_ttyname() to avoid some
extra work and having to catch the ENODEV.

* libguile/fports.c: include the integer fd when ttyname() fails with
ENODEV.
* libguile/posix.c: add scm_i_c_ttyname().
* libguile/posix.h: add scm_i_c_ttyname().
---
 configure.ac      |  2 +-
 libguile/fports.c | 31 ++++++++++++++++---
 libguile/posix.c  | 78 ++++++++++++++++++++++++++++-------------------
 libguile/posix.h  |  1 +
 4 files changed, 74 insertions(+), 38 deletions(-)

diff --git a/configure.ac b/configure.ac
index af9176b46..2e89f93a9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -549,7 +549,7 @@ AC_CHECK_FUNCS([DINFINITY DQNAN cexp chsize clog clog10 ctermid         \
   nice readlink rmdir setegid seteuid                            \
   setuid setgid setpgid setsid sigaction siginterrupt stat64  \
   strptime symlink sync sysconf tcgetpgrp tcsetpgrp uname waitpid       \
-  strdup usleep on_exit chown link fcntl ttyname getpwent \
+  strdup usleep on_exit chown link fcntl ttyname ttyname_r getpwent \
   getgrent kill getppid getpgrp fork setitimer getitimer strchr strcmp  \
   index bcopy rindex truncate isblank _NSGetEnviron              \
   strcoll_l strtod_l strtol_l newlocale uselocale utimensat     \
diff --git a/libguile/fports.c b/libguile/fports.c
index 9d4ca6ace..5ba09c552 100644
--- a/libguile/fports.c
+++ b/libguile/fports.c
@@ -554,6 +554,7 @@ SCM_DEFINE (scm_adjust_port_revealed_x, "adjust-port-revealed!", 2, 0, 0,
 
 
 
+#define FUNC_NAME "fport_print"
 static int 
 fport_print (SCM exp, SCM port, scm_print_state *pstate SCM_UNUSED)
 {
@@ -570,12 +571,31 @@ fport_print (SCM exp, SCM port, scm_print_state *pstate SCM_UNUSED)
       scm_putc (' ', port);
       fdes = (SCM_FSTREAM (exp))->fdes;
 
-#if (defined HAVE_TTYNAME) && (defined HAVE_POSIX)
-      if (isatty (fdes))
-	scm_display (scm_ttyname (exp), port);
-      else
-#endif /* HAVE_TTYNAME */
+#if ((defined HAVE_TTYNAME) || (defined HAVE_TTYNAME_R)) && (defined HAVE_POSIX)
+      if (!isatty (fdes))
 	scm_intprint (fdes, 10, port);
+      else
+        {
+          SCM name = scm_i_c_ttyname (fdes);
+          if (scm_is_string (name))
+            scm_display (name, port);
+          else
+            {
+              const int err = scm_to_int (name);
+              // In some situations, ttyname may return ENODEV even
+              // though isatty is true.  cf. the GNU/Linux ttyname(3).
+              if (err == ENODEV)
+                scm_intprint (fdes, 10, port);
+              else
+                {
+                  errno = err;
+                  SCM_SYSERROR;
+                }
+            }
+        }
+#else /* can't use ttyname */
+      scm_intprint (fdes, 10, port);
+#endif
     }
   else
     {
@@ -586,6 +606,7 @@ fport_print (SCM exp, SCM port, scm_print_state *pstate SCM_UNUSED)
   scm_putc ('>', port);
   return 1;
 }
+#undef FUNC_NAME
 
 /* fill a port's read-buffer with a single read.  returns the first
    char or EOF if end of file.  */
diff --git a/libguile/posix.c b/libguile/posix.c
index 0e57f012b..a9f726f98 100644
--- a/libguile/posix.c
+++ b/libguile/posix.c
@@ -1043,52 +1043,66 @@ SCM_DEFINE (scm_getsid, "getsid", 1, 0, 0,
 #endif /* HAVE_GETSID */
 
 
-/* ttyname returns its result in a single static buffer, hence
-   scm_i_misc_mutex for thread safety.  In glibc 2.3.2 two threads
-   continuously calling ttyname will otherwise get an overwrite quite
-   easily.
+#if (defined HAVE_TTYNAME) || (defined HAVE_TTYNAME_R)
 
-   ttyname_r (when available) could be used instead of scm_i_misc_mutex, but
-   there's probably little to be gained in either speed or parallelism.  */
+#define FUNC_NAME "scm_i_c_ttyname"
+SCM
+scm_i_c_ttyname (int fd)
+{
+  // Return the string ttyname or the integer errno.
+  int err = 0;
+  char name[TTY_NAME_MAX];
+
+#ifdef HAVE_TTYNAME_R
+  SCM_SYSCALL (err = ttyname_r (fd, name, TTY_NAME_MAX));
+#else // HAVE_TTYNAME
+  // ttyname returns its result in a single static buffer, hence
+  // scm_i_misc_mutex for thread safety.  In glibc 2.3.2 two threads
+  // continuously calling ttyname will otherwise get an overwrite quite
+  // easily.
+  scm_i_scm_pthread_mutex_lock (&scm_i_misc_mutex);
+  char *global_name;
+  SCM_SYSCALL (global_name = ttyname (fd));
+  err = errno;
+  if (!err)
+    {
+      // Not necessary if ttyname() must also respect TTY_NAME_MAX.
+      // POSIX ttyname description isn't completely clear.
+      if (strlen (global_name) > TTY_NAME_MAX - 1)
+        errno = ERANGE;
+      else
+        strcpy(name, global_name);
+    }
+  scm_i_pthread_mutex_unlock (&scm_i_misc_mutex);
+#endif // HAVE_TTYNAME
 
-#ifdef HAVE_TTYNAME
-SCM_DEFINE (scm_ttyname, "ttyname", 1, 0, 0, 
+  if (err)
+    return scm_from_int(err);
+  else
+    return scm_from_locale_string (name);
+}
+#undef FUNC_NAME
+
+SCM_DEFINE (scm_ttyname, "ttyname", 1, 0, 0,
             (SCM port),
 	    "Return a string with the name of the serial terminal device\n"
 	    "underlying @var{port}.")
 #define FUNC_NAME s_scm_ttyname
 {
-  char *result;
-  int fd, err;
-  SCM ret = SCM_BOOL_F;
-
   port = SCM_COERCE_OUTPORT (port);
   SCM_VALIDATE_OPPORT (1, port);
   if (!SCM_FPORTP (port))
     return SCM_BOOL_F;
-  fd = SCM_FPORT_FDES (port);
 
-  scm_i_scm_pthread_mutex_lock (&scm_i_misc_mutex);
-
-  SCM_SYSCALL (result = ttyname (fd));
-  err = errno;
-  if (result != NULL)
-    result = strdup (result);
-
-  scm_i_pthread_mutex_unlock (&scm_i_misc_mutex);
-
-  if (!result)
-    {
-      errno = err;
-      SCM_SYSERROR;
-    }
-  else
-    ret = scm_take_locale_string (result);
-
-  return ret;
+  SCM result = scm_i_c_ttyname (SCM_FPORT_FDES (port));
+  if (scm_is_string (result))
+    return result;
+  errno = scm_to_int (result);
+  SCM_SYSERROR;
 }
 #undef FUNC_NAME
-#endif /* HAVE_TTYNAME */
+
+#endif // (defined HAVE_TTYNAME) || (defined HAVE_TTYNAME_R)
 
 
 /* For thread safety "buf" is used instead of NULL for the ctermid static
diff --git a/libguile/posix.h b/libguile/posix.h
index a4b0297b3..acfaa3c70 100644
--- a/libguile/posix.h
+++ b/libguile/posix.h
@@ -91,6 +91,7 @@ SCM_API SCM scm_sethostname (SCM name);
 SCM_API SCM scm_gethostname (void);
 SCM_API SCM scm_getaffinity (SCM pid);
 SCM_API SCM scm_setaffinity (SCM pid, SCM cpu_set);
+SCM_INTERNAL SCM scm_i_c_ttyname (int fd);
 SCM_INTERNAL void scm_init_posix (void);
 
 SCM_INTERNAL scm_i_pthread_mutex_t scm_i_locale_mutex;
-- 
2.45.2