bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

test-open failure on Hurd


From: Eric Blake
Subject: test-open failure on Hurd
Date: Fri, 02 Oct 2009 18:32:20 -0600
User-agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.23) Gecko/20090812 Thunderbird/2.0.0.23 Mnenhy/0.7.6.666

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

[adding bug-gnulib]

According to Samuel Thibault on 10/2/2009 3:36 PM:
> Eric Blake, le Fri 02 Oct 2009 15:32:01 -0600, a écrit :
>> http://www.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap10.html
> 
> Yep, that's what I was reading.
> 
>> Although some places in POSIX use the term "file" to mean any directory
>> entry, this particular division makes it seem pretty clear to me that
>> /dev/null is not a directory.
> 
> It says 
> 
> “An infinite data source and data sink. Data written to /dev/null shall be
> discarded. Reads from /dev/null shall always return end-of-file (EOF).”
> 
> It happens that on the hurd, a directory can be a data source too, so
> there is no contradiction.

Actually, there is a contradiction.  You've covered the data source just
fine with a directory, but not the data sink.  open(dir,O_WRONLY) is
required to fail with EISDIR, but open("/dev/null",O_WRONLY) is required
to succeed.

[and for those reading this thread, yes, the Austin group has corrected
the typo in POSIX 2008 - /dev/null is not an infinite data source, but "An
empty data source and infinite data sink."]
http://austingroupbugs.net/view.php?id=134

> 
>>> Anyway, as I said, the status of such resolution has changed over time
>>> and it's probably a matter of fixing it, it's just that nobody has yet
>>> taken the time to do it, that's why I had just dropped the test for now
>>> and not submitted any patch, it's just a quickfix.
>> Meanwhile, gnulib should probably be more tolerant, and figure out how to
>> work around this problem gracefully rather than failing.
> 
> I don't know why they explicitly check for /dev/null/ indeed.

I'll let others decide how Hurd will fix /dev/null, or if a fix is even
necessary.  But meanwhile, the point of test-open was to test that opening
a non-directory with a trailing slash fails, not whether Hurd complies.
Therefore, we can fix the testsuite to be more portable by testing
something other than /dev/null/.  I'm checking this in (although I have
not yet tested whether it improves the situation on Hurd):

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkrGm5QACgkQ84KuGfSFAYAcvwCgwu/en1J6SoiDwjO6o00oDFDi
iWgAoK582222wwtYUrO9aKOLHsPU7G5J
=kF9z
-----END PGP SIGNATURE-----
From 6538bb09a27d32e66e1977b7941b228c06f03e14 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Fri, 2 Oct 2009 18:06:21 -0600
Subject: [PATCH] test-open: on GNU/Hurd, /dev/null is a directory

* tests/test-fopen.h (main): Rename...
(test_fopen): ...to this.  Use a guaranteed non-directory when
confirming open behavior on trailing slash.
* tests/test-openat-safer.c (main): Likewise.
* tests/test-open.h (main): Likewise....
(test_open): ...to this.
* tests/test-fopen.c (main): Adjust caller.
* tests/test-fopen-safer.c (main): Likewise.
* tests/test-open.c (main): Likewise.
* tests/test-fcntl-safer.c (main): Likewise.
Reported by Samuel Thibault.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                 |   13 ++++++++++++
 tests/test-fcntl-safer.c  |    8 +++++++
 tests/test-fopen-safer.c  |    8 +++++++
 tests/test-fopen.c        |    8 +++++++
 tests/test-fopen.h        |   47 +++++++++++++++++++++++++++++++++++++++---
 tests/test-open.c         |    8 +++++++
 tests/test-open.h         |   49 ++++++++++++++++++++++++++++++++++++++++----
 tests/test-openat-safer.c |   12 +++++-----
 8 files changed, 138 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1125c58..3ef349e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
 2009-10-02  Eric Blake  <address@hidden>

+       test-open: on GNU/Hurd, /dev/null is a directory
+       * tests/test-fopen.h (main): Rename...
+       (test_fopen): ...to this.  Use a guaranteed non-directory when
+       confirming open behavior on trailing slash.
+       * tests/test-openat-safer.c (main): Likewise.
+       * tests/test-open.h (main): Likewise....
+       (test_open): ...to this.
+       * tests/test-fopen.c (main): Adjust caller.
+       * tests/test-fopen-safer.c (main): Likewise.
+       * tests/test-open.c (main): Likewise.
+       * tests/test-fcntl-safer.c (main): Likewise.
+       Reported by Samuel Thibault.
+
        rename, fchdir: don't ignore chdir failure
        * lib/fchdir.c (get_name): Abort on unexpected chdir failure.
        * lib/rename.c (rpl_rename) [W32]: Likewise.
diff --git a/tests/test-fcntl-safer.c b/tests/test-fcntl-safer.c
index 3b3ff75..433160b 100644
--- a/tests/test-fcntl-safer.c
+++ b/tests/test-fcntl-safer.c
@@ -20,4 +20,12 @@

 #include "fcntl--.h"

+#define BASE "test-fcntl-safer.t"
+
 #include "test-open.h"
+
+int
+main ()
+{
+  return test_open ();
+}
diff --git a/tests/test-fopen-safer.c b/tests/test-fopen-safer.c
index 701af35..e613364 100644
--- a/tests/test-fopen-safer.c
+++ b/tests/test-fopen-safer.c
@@ -20,4 +20,12 @@

 #include "stdio--.h"

+#define BASE "test-fopen-safer.t"
+
 #include "test-fopen.h"
+
+int
+main ()
+{
+  return test_fopen ();
+}
diff --git a/tests/test-fopen.c b/tests/test-fopen.c
index 473d274..6efd480 100644
--- a/tests/test-fopen.c
+++ b/tests/test-fopen.c
@@ -20,4 +20,12 @@

 #include <stdio.h>

+#define BASE "test-fopen.t"
+
 #include "test-fopen.h"
+
+int
+main ()
+{
+  return test_fopen ();
+}
diff --git a/tests/test-fopen.h b/tests/test-fopen.h
index b1dafbb..02b30ca 100644
--- a/tests/test-fopen.h
+++ b/tests/test-fopen.h
@@ -18,7 +18,9 @@

 /* Include <config.h> and a form of <stdio.h> first.  */

+#include <errno.h>
 #include <stdlib.h>
+#include <unistd.h>

 #define ASSERT(expr) \
   do                                                                        \
@@ -32,13 +34,50 @@
     }                                                                       \
   while (0)

-int
-main ()
+/* Test fopen.  Assumes BASE is defined.  */
+
+static int
+test_fopen (void)
 {
+  FILE *f;
+  /* Remove anything from prior partial run.  */
+  unlink (BASE "file");
+
+  /* Read requires existing file.  */
+  errno = 0;
+  ASSERT (fopen (BASE "file", "r") == NULL);
+  ASSERT (errno == ENOENT);
+
+  /* Write can create a file.  */
+  f = fopen (BASE "file", "w");
+  ASSERT (f);
+  ASSERT (fclose (f) == 0);
+
+  /* Trailing slash is invalid on non-directory.  */
+  errno = 0;
+  ASSERT (fopen (BASE "file/", "r") == NULL);
+  ASSERT (errno == ENOTDIR || errno == EISDIR);
+
+  /* Cannot create a directory.  */
+  errno = 0;
   ASSERT (fopen ("nonexist.ent/", "w") == NULL);
-  ASSERT (fopen ("/dev/null/", "r") == NULL);
+  ASSERT (errno == ENOTDIR || errno == EISDIR || errno == ENOENT);
+
+  /* Directories cannot be opened for writing.  */
+  errno = 0;
+  ASSERT (fopen (".", "w") == NULL);
+  ASSERT (errno == EISDIR);
+
+  /* /dev/null must exist, and be writable.  */
+  f = fopen ("/dev/null", "r");
+  ASSERT (f);
+  ASSERT (fclose (f) == 0);
+  f = fopen ("/dev/null", "w");
+  ASSERT (f);
+  ASSERT (fclose (f) == 0);

-  ASSERT (fopen ("/dev/null", "r") != NULL);
+  /* Cleanup.  */
+  ASSERT (unlink (BASE "file") == 0);

   return 0;
 }
diff --git a/tests/test-open.c b/tests/test-open.c
index df7e36f..6b97e18 100644
--- a/tests/test-open.c
+++ b/tests/test-open.c
@@ -20,4 +20,12 @@

 #include <fcntl.h>

+#define BASE "test-open.t"
+
 #include "test-open.h"
+
+int
+main ()
+{
+  return test_open ();
+}
diff --git a/tests/test-open.h b/tests/test-open.h
index 466cab3..7381037 100644
--- a/tests/test-open.h
+++ b/tests/test-open.h
@@ -18,8 +18,10 @@

 /* Include <config.h> and a form of <fcntl.h> first.  */

+#include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
+#include <unistd.h>

 #define ASSERT(expr) \
   do                                                                        \
@@ -33,13 +35,50 @@
     }                                                                       \
   while (0)

-int
-main ()
+/* Test fopen.  Assumes BASE is defined.  */
+
+static int
+test_open (void)
 {
-  ASSERT (open ("nonexist.ent/", O_CREAT | O_RDONLY, 0600) < 0);
-  ASSERT (open ("/dev/null/", O_RDONLY) < 0);
+  int fd;
+  /* Remove anything from prior partial run.  */
+  unlink (BASE "file");
+
+  /* Cannot create directory.  */
+  errno = 0;
+  ASSERT (open ("nonexist.ent/", O_CREAT | O_RDONLY, 0600) == -1);
+  ASSERT (errno == ENOTDIR || errno == EISDIR || errno == ENOENT);
+
+  /* Create a regular file.  */
+  fd = open (BASE "file", O_CREAT | O_RDONLY, 0600);
+  ASSERT (0 <= fd);
+  ASSERT (close (fd) == 0);
+
+  /* Trailing slash handling.  */
+  errno = 0;
+  ASSERT (open (BASE "file/", O_RDONLY) == -1);
+  ASSERT (errno == ENOTDIR || errno == EISDIR);
+
+  /* Directories cannot be opened for writing.  */
+  errno = 0;
+  ASSERT (open (".", O_WRONLY) == -1);
+  ASSERT (errno == EISDIR);
+
+  /* /dev/null must exist, and be writable.  */
+  fd = open ("/dev/null", O_RDONLY);
+  ASSERT (0 <= fd);
+  {
+    char c;
+    ASSERT (read (fd, &c, 1) == 0);
+  }
+  ASSERT (close (fd) == 0);
+  fd = open ("/dev/null", O_WRONLY);
+  ASSERT (0 <= fd);
+  ASSERT (write (fd, "c", 1) == 1);
+  ASSERT (close (fd) == 0);

-  ASSERT (open ("/dev/null", O_RDONLY) >= 0);
+  /* Cleanup.  */
+  ASSERT (unlink (BASE "file") == 0);

   return 0;
 }
diff --git a/tests/test-openat-safer.c b/tests/test-openat-safer.c
index 47d3ada..7e44d67 100644
--- a/tests/test-openat-safer.c
+++ b/tests/test-openat-safer.c
@@ -45,6 +45,8 @@ static FILE *myerr;
     }                                                                        \
   while (0)

+#define witness "test-openat-safer.txt"
+
 int
 main ()
 {
@@ -53,7 +55,6 @@ main ()
   int dfd;
   int fd;
   char buf[2];
-  const char *witness = "test-openat-safer.txt";

   /* We close fd 2 later, so save it in fd 10.  */
   if (dup2 (STDERR_FILENO, BACKUP_STDERR_FILENO) != BACKUP_STDERR_FILENO
@@ -96,15 +97,14 @@ main ()
          ASSERT (openat (-1, ".", O_RDONLY) == -1);
          ASSERT (errno == EBADF);

-         /* Check for trailing slash and /dev/null handling; the
-            particular errno might be ambiguous.  */
+         /* Check for trailing slash and /dev/null handling.  */
          errno = 0;
          ASSERT (openat (dfd, "nonexist.ent/", O_CREAT | O_RDONLY,
                          S_IRUSR | S_IWUSR) == -1);
-         /* ASSERT (errno == ENOTDIR); */
+         ASSERT (errno == ENOTDIR || errno == EISDIR || errno == ENOENT);
          errno = 0;
-         ASSERT (openat (dfd, "/dev/null/", O_RDONLY) == -1);
-         /* ASSERT (errno == ENOTDIR); */
+         ASSERT (openat (dfd, witness "/", O_RDONLY) == -1);
+         ASSERT (errno == ENOTDIR || errno == EISDIR);
          /* Using a bad directory is okay for absolute paths.  */
          fd = openat (-1, "/dev/null", O_WRONLY);
          ASSERT (STDERR_FILENO < fd);
-- 
1.6.5.rc1


reply via email to

[Prev in Thread] Current Thread [Next in Thread]