Re: grantpt test

[Bruno Haible]

Rich Felker wrote:
> > test-grantpt.c:34: assertion failed
> > FAIL: test-grantpt
> 
> This is an invalid test. POSIX specifies this function "may fail", not
> "shall fail", and since the function is inherently a no-op, it would
> be idiotic to make it perform a syscall to check the validity of the
> file descriptor...

Looking at the (few) callers of grantpt() in gnulib, it indeed seems
unlikely that people will want to rely on the failure for invalid file
descriptors. So I'm relaxing the requirements of gnulib.


2012-06-22  Bruno Haible  <address@hidden>

        grantpt: Relax requirement regarding invalid file descriptors.
        * lib/grantpt.c: Don't include <fcntl.h>.
        (grantpt): Don't verify the validity of the file descriptor.
        * modules/grantpt (Depends-on): Remove fcntl-h.
        * tests/test-grantpt.c (main): Allow grantpt to succeed for invalid
        file descriptors.
        * doc/posix-functions/grantpt.texi: Document more platforms on which
        grantpt succeeds for invalid file descriptors.
        Reported by Rich Felker <address@hidden>.

--- doc/posix-functions/grantpt.texi.orig       Fri Jun 22 12:33:55 2012
+++ doc/posix-functions/grantpt.texi    Fri Jun 22 12:33:52 2012
@@ -20,5 +20,5 @@
 IRIX 5.3.
 @item
 This function reports success for invalid file descriptors on some platforms:
-Cygwin 1.7.9.
+OpenBSD, Cygwin 1.7.9, musl libc.
 @end itemize
--- lib/grantpt.c.orig  Fri Jun 22 12:33:55 2012
+++ lib/grantpt.c       Fri Jun 22 12:14:57 2012
@@ -21,7 +21,6 @@
 
 #include <assert.h>
 #include <errno.h>
-#include <fcntl.h>
 #include <string.h>
 #include <sys/wait.h>
 #include <unistd.h>
@@ -50,8 +49,6 @@
 #if defined __OpenBSD__
   /* On OpenBSD, master and slave of a pseudo-terminal are allocated together,
      through an ioctl on /dev/ptm.  There is no need for grantpt().  */
-  if (fcntl (fd, F_GETFD) < 0)
-    return -1;
   return 0;
 #else
   /* This function is most often called from a process without 'root'
--- modules/grantpt.orig        Fri Jun 22 12:33:55 2012
+++ modules/grantpt     Fri Jun 22 12:15:14 2012
@@ -9,7 +9,6 @@
 Depends-on:
 stdlib
 extensions
-fcntl-h         [test $HAVE_GRANTPT = 0]
 pt_chown        [test $HAVE_GRANTPT = 0]
 waitpid         [test $HAVE_GRANTPT = 0]
 configmake      [test $HAVE_GRANTPT = 0]
--- tests/test-grantpt.c.orig   Fri Jun 22 12:33:55 2012
+++ tests/test-grantpt.c        Fri Jun 22 12:14:31 2012
@@ -28,22 +28,36 @@
 int
 main (void)
 {
-  /* Test behaviour for invalid file descriptors.  */
+  /* Test behaviour for invalid file descriptors.
+     These calls don't fail on OpenBSD (with gnulib's replacement) and on
+     musl libc.  */
   {
+    int ret;
+
     errno = 0;
-    ASSERT (grantpt (-1) == -1);
-    ASSERT (errno == EBADF
-            || errno == EINVAL /* seen on FreeBSD 6.4 */
-            || errno == 0 /* seen on Solaris 8 */
-           );
+    ret = grantpt (-1);
+    if (ret != 0)
+      {
+        ASSERT (ret == -1);
+        ASSERT (errno == EBADF
+                || errno == EINVAL /* seen on FreeBSD 6.4 */
+                || errno == 0 /* seen on Solaris 8 */
+               );
+      }
   }
   {
+    int ret;
+
     errno = 0;
-    ASSERT (grantpt (99) == -1);
-    ASSERT (errno == EBADF
-            || errno == EINVAL /* seen on FreeBSD 6.4 */
-            || errno == 0 /* seen on Solaris 8 */
-           );
+    ret = grantpt (99);
+    if (ret != 0)
+      {
+        ASSERT (ret == -1);
+        ASSERT (errno == EBADF
+                || errno == EINVAL /* seen on FreeBSD 6.4 */
+                || errno == 0 /* seen on Solaris 8 */
+               );
+      }
   }
 
   return 0;