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;
gnulib uses it only to implement a replacement openpty() if
the system lacks both openpty() and granpt(), but all
systems that we currently port to either have at least one of
openpty() and/or grantpt(), or lack ptys altogether.  That is,
we aren't porting to any system that requires us to deal with
the hassle of installing a setuid pt_chown helper just to use
gnulib's ability to provide openpty() on obscure 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.

.gnulib

@@ -1 +1 @@
-Subproject commit 2394a603e7586e671226478e5b15d924c3841f42
+Subproject commit 0031e4f6353cc7077a9d0dad0c793bd6e3dc7aaa

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.

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"
@@ -1172,52 +1174,85 @@ virFileBuildPath(const char *dir, const char *name, const char *ext)
     return path;
 }
 
+/* Open a non-blocking master side of a pty.  If ttyName is not NULL,
+ * then populate it with the name of the slave.  If rawmode is set,
+ * also put the master side into raw mode before returning.  */
 #ifndef WIN32
 int virFileOpenTty(int *ttymaster,
                    char **ttyName,
                    int rawmode)
 {
-    int rc = -1;
+    /* XXX A word of caution - on some platforms (Solaris and HP-UX),
+     * additional ioctl() calls are needs after opening the slave
+     * before it will cause isatty() to return true.  Should we make
+     * virFileOpenTty also return the opened slave fd, so the caller
+     * doesn't have to worry about that mess?  */
+    int ret = -1;
+    int slave = -1;
+    char *name = NULL;
 
-    if ((*ttymaster = posix_openpt(O_RDWR|O_NOCTTY|O_NONBLOCK)) < 0)
-        goto cleanup;
-
-    if (unlockpt(*ttymaster) < 0)
-        goto cleanup;
-
-    if (grantpt(*ttymaster) < 0)
+    /* 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;
+
+    /* What a shame that openpty cannot atomically set FD_CLOEXEC, but
+     * that using posix_openpt/grantpt/unlockpt/ptsname is not
+     * thread-safe, and that ptsname_r is not portable.  */
+    if (virSetNonBlock(*ttymaster) < 0 ||
+        virSetCloseExec(*ttymaster) < 0)
         goto cleanup;
 
+    /* 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;
+
+        while ((rc = ttyname_r(slave, name, len)) == ERANGE) {
+            if (VIR_RESIZE_N(name, len, len, len) < 0)
                 goto cleanup;
         }
-
-        if (ptsname_r(*ttymaster, *ttyName, PATH_MAX) != 0) {
-            VIR_FREE(*ttyName);
+        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,

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);