Rob Browning <r...@defaultvalue.org> writes:
> Oh, and in addition to wondering about using ttyname_r when it's
> available, if we did that, or even if we didn't, I could also see moving
> the shared code to a shared helper, but I was just starting with a
> "simpler" proposal (to avoid the crash).
Attached is a variant of the fancier approach (also relying on ttyname_r
when available) that I'd been thinking of (though might not be quite
right yet), and which I realized you probably also suggested.
The TTY_NAME_MAX comment's referring to whether or not "The maximum
length of the terminal name shall be {TTY_NAME_MAX}." here only applies
to the paragraph it's in (i.e. just for ttyname_r()), or applies to
ttyname() too:
https://pubs.opengroup.org/onlinepubs/009696899/functions/ttyname.html
>From 94b338f144441483b0d0513be5c90d88c7aaeffb Mon Sep 17 00:00:00 2001
From: Rob Browning <r...@defaultvalue.org>
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
--
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4
