[libvirt] [PATCH] build: fix build on platforms without ptsname_r

MacOS lacks ptsname_r, and gnulib doesn't (yet) provide it. But we can avoid it altogether, by using gnulib openpty() instead. Note that we do _not_ want the pt_chown module; all systems that we currently port to can either properly do openpty() and/or grantpt(), or lack ptys altogether; we are not porting to any system that requires us to deal with the hassle of installing a setuid pt_chown helper just to satisfy gnulib's ability to provide openpty() on even more platforms. * .gnulib: Update to latest, for openpty fixes * bootstrap.conf (gnulib_modules): Add openpty, ttyname_r. (gnulib_tool_option_extras): Exclude pt_chown module. * src/util/util.c (virFileOpenTty): Rewrite in terms of openpty and ttyname_r. * src/util/util.h (virFileOpenTtyAt): Delete dead prototype. Reported by Justin Clift. --- I compile-tested this on Linux, but haven't yet tested this on MacOS, which is the driving force behind this patch. Meanwhile, it would be nice to run some LXC tests to ensure this all still works. I'm posting it now, to get the review started while I figure out how to run further tests. Gnulib changes are: * .gnulib 2394a60...84c3f9c (78): .gnulib | 2 +- bootstrap.conf | 3 ++ src/util/util.c | 62 ++++++++++++++++++++++++++++++++++++------------------ src/util/util.h | 5 ---- 4 files changed, 45 insertions(+), 27 deletions(-) diff --git a/.gnulib b/.gnulib index 2394a60..84c3f9c 160000 --- a/.gnulib +++ b/.gnulib @@ -1 +1 @@ -Subproject commit 2394a603e7586e671226478e5b15d924c3841f42 +Subproject commit 84c3f9cf54eaa688c5a1a26925886535079a91de diff --git a/bootstrap.conf b/bootstrap.conf index 0faa2e2..4557d2d 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -68,6 +68,7 @@ mkstemps mktempd netdb nonblocking +openpty passfd perror physmem @@ -101,6 +102,7 @@ sys_wait termios time_r timegm +ttyname_r uname useless-if-before-free usleep @@ -168,6 +170,7 @@ tests_base=gnulib/tests gnulib_tool_option_extras="\ --lgpl=2\ --with-tests\ + --avoid=pt_chown\ " # Convince bootstrap to use multiple m4 directories. diff --git a/src/util/util.c b/src/util/util.c index bd52b21..5130403 100644 --- a/src/util/util.c +++ b/src/util/util.c @@ -45,10 +45,11 @@ #include <string.h> #include <signal.h> #include <termios.h> +#include <pty.h> + #if HAVE_LIBDEVMAPPER_H # include <libdevmapper.h> #endif -#include "c-ctype.h" #ifdef HAVE_PATHS_H # include <paths.h> @@ -65,6 +66,7 @@ # include <mntent.h> #endif +#include "c-ctype.h" #include "dirname.h" #include "virterror_internal.h" #include "logging.h" @@ -1177,47 +1179,65 @@ int virFileOpenTty(int *ttymaster, char **ttyName, int rawmode) { - int rc = -1; - - if ((*ttymaster = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK)) < 0) - goto cleanup; - - if (unlockpt(*ttymaster) < 0) - goto cleanup; + int ret = -1; + int slave = -1; + char *name = NULL; - if (grantpt(*ttymaster) < 0) - goto cleanup; + /* Unfortunately, we can't use the name argument of openpty, since + * there is no guarantee on how large the buffer has to be. + * Likewise, we can't use the termios argument: we have to use + * read-modify-write since there is no portable way to initialize + * a struct termios without use of tcgetattr. */ + if (openpty(ttymaster, &slave, NULL, NULL, NULL) < 0) + return -1; + /* While Linux supports tcgetattr on either the master or the + * slave, Solaris requires it to be on the slave. */ if (rawmode) { struct termios ttyAttr; - if (tcgetattr(*ttymaster, &ttyAttr) < 0) + if (tcgetattr(slave, &ttyAttr) < 0) goto cleanup; cfmakeraw(&ttyAttr); - if (tcsetattr(*ttymaster, TCSADRAIN, &ttyAttr) < 0) + if (tcsetattr(slave, TCSADRAIN, &ttyAttr) < 0) goto cleanup; } + /* ttyname_r on the slave is required by POSIX, while ptsname_r on + * the master is a glibc extension, and the POSIX ptsname is not + * thread-safe. Since openpty gave us both descriptors, guess + * which way we will determine the name? :) */ if (ttyName) { - if (VIR_ALLOC_N(*ttyName, PATH_MAX) < 0) { - errno = ENOMEM; + /* Initial guess of 64 is generally sufficient; rely on ERANGE + * to tell us if we need to grow. */ + size_t len = 64; + int rc; + + if (VIR_ALLOC_N(name, len) < 0) goto cleanup; - } - if (ptsname_r(*ttymaster, *ttyName, PATH_MAX) != 0) { - VIR_FREE(*ttyName); + while ((rc = ttyname_r(slave, name, len)) == ERANGE) { + if (VIR_RESIZE_N(name, len, len, len) < 0) + goto cleanup; + } + if (rc != 0) { + errno = rc; goto cleanup; } + *ttyName = name; + name = NULL; } - rc = 0; + ret = 0; cleanup: - if (rc != 0) + if (ret != 0) VIR_FORCE_CLOSE(*ttymaster); + VIR_FORCE_CLOSE(slave); + VIR_FREE(name); - return rc; + return ret; } #else /* WIN32 */ int virFileOpenTty(int *ttymaster ATTRIBUTE_UNUSED, diff --git a/src/util/util.h b/src/util/util.h index e869f1b..d8176a8 100644 --- a/src/util/util.h +++ b/src/util/util.h @@ -121,11 +121,6 @@ int virFileAbsPath(const char *path, int virFileOpenTty(int *ttymaster, char **ttyName, int rawmode); -int virFileOpenTtyAt(const char *ptmx, - int *ttymaster, - char **ttyName, - int rawmode); - char *virArgvToString(const char *const *argv); -- 1.7.4.4