Re: grantpt test

[Bruno Haible <bruno@clisp.org>]

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  <bruno@clisp.org>

	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 <dalias@aerifal.cx>.

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