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):
> New module 'unlinkat', split off from module 'openat'.
> New module 'fchmodat', split off from module 'openat'.
> putenv: indent #definition of "environ" to placate cppi
> gitlog-to-changelog: provide a ChangeLog-repair mechanism
> gitlog-to-changelog: avoid an infloop
> * MODULES.html.sh: Fix sed-script shell quoting and locale issues.
> fchownat: Improve description.
> * tests/test-stdalign.c (TEST_ALIGNMENT): Shrink back to 8.
> Fix my old ChangeLog entry to properly cite Bruno's email.
> alignof: Avoid collision with stdalign module.
> New module 'fchownat', split off from module 'openat'.
> stdalign: port better to MSVC and to Sun C 5.11
> doc about some IRIX 5.3 problems.
> gitlog-to-changelog: fix git-log invocation
> gitlog-to-changelog: new option --append-dot
> ffsl, ffsll: Avoid compilation error due to 'restrict'.
> GNUmakefile: reenable "make syntax-check" for most projects
> gitlog-to-changelog: treat a message with only blank lines as empty
> test-parse-datetime.c: avoid new DST-related false positive test failure
> autoupdate
> fstat: Tweak documentation.
> Update documentation regarding 'largefile' module.
> maint.mk: don't maintain a second build-aux variable.
> Adjust to Bruno's comments.
> sys_socket: use stdalign, not alignof
> crypto libraries: use stdalign
> argp: use stdalign
> stdalign-tests: new module
> stdalign: new module
> raise test: Avoid a test failure on Linux/MIPS.
> nonblocking tests: Fix test failure on Linux/MIPS.
> utimensat: Work around problem on Linux/hppa.
> maint.mk: fix a bug in sc_prohibit_stddef_without_use
> maint.mk: exempt ENODATA from a syntax-check rule
> fts: close parent dir FD before returning from post-traversal fts_read
> autoupdate
> readme-release: improve safety of release prep instructions.
> errno, strerror-override: Support for MSVC 10.
> perror: Recognize when test program crashes.
> perror: Fix indentation.
> isfinite, isinf, isnan, signbit: Don't define as a macro in C++.
> relocatable-prog-wrapper: Don't leave object files behind.
> openpty, posix_openpt: Remove code duplication.
> unlockpt: Detect invalid argument.
> openpty: Avoid compilation error on AIX 6.1.
> autoupdate
> posix_openpt: Support for OpenBSD.
> posix_openpt test: Coding style.
> grantpt: Support --avoid=pt_chown.
> posix_openpt: Fix autoconf macro.
> openpty: Update comments.
> openpty: relax license
> pt_chown: use configmake to simplify build
> ptsname and others: relax license
> update from texinfo
> posix_openpt: remove spurious #endif
> maint.mk: Respect $(build_aux) in web-manual rule.
> posix_openpt: Fix compilation error.
> Support for old NeXTstep 3.3 frexp().
> Support for old NeXTstep 3.3 sed.
> Support for old NeXTstep 3.3 gcc.
> posix_openpt: new module
> xstrtoll: Fix compilation failure.
> vasnprintf: Optimize bit search operation.
> vasnprintf: Fix comments.
> Tests for module 'integer_length_ll'.
> New module 'integer_length_ll'.
> Tests for module 'integer_length_l'.
> New module 'integer_length_l'.
> Tests for module 'integer_length'.
> New module 'integer_length'.
> popen: Fix dependency conditions.
> perror: Fix autoconf test.
> ffsl: Optimize on 64-bit platforms.
> autoupdate
> ffsl: Optimize on 32-bit platforms.
> ffsl, ffsll: Optimize for GCC.
> ffs, bcopy, memset: Support symbol renaming via config.h.
.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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list