bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
* supersede: Avoid a failure when writing to /dev/null in Solaris zones
@ 2020-08-23 17:38 Bruno Haible
  2020-09-20 19:31 ` Bruno Haible
  0 siblings, 1 reply; 3+ messages in thread
From: Bruno Haible @ 2020-08-23 17:38 UTC (permalink / raw)
  To: bug-gnulib; +Cc: Jörg Sonnenberger, Thomas Klausner

Jörg Sonnenberger noticed that the new gnulib module 'supersede', when
writing to /dev/null in Solaris/Illumos zones, fails [1][2].

Per POSIX [3],
  open ("/dev/null", O_TRUNC | O_WRONLY)
and
  open ("/dev/null", O_CREAT | O_TRUNC | O_WRONLY)
should be equivalent. But in Solaris/Illumos zones, the second form is needed
because the first one fails with EINVAL.

Should we extend the 'open' module to cover this bug? Probably overkill,
especially since I can't see how we could test for it in an autoconf test.

Here's a workaround, specifically in the 'supersede' module.

[1] https://www.illumos.org/issues/13035
[2] https://pkgsrc.se/files.php?messageId=20200812233110.30230FB28@cvs.NetBSD.org
[3] https://pubs.opengroup.org/onlinepubs/9699919799.2018edition/functions/open.html


2020-08-23  Bruno Haible  <bruno@clisp.org>

	supersede: Avoid a failure when writing to /dev/null in Solaris zones.
	Reported by Jörg Sonnenberger <joerg@netbsd.org>
	via Thomas Klausner <tk@giga.or.at> in
	<https://pkgsrc.se/files.php?messageId=20200812233110.30230FB28@cvs.NetBSD.org>.
	* lib/supersede.c (open_supersede): When opening an existing non-regular
	file on Solaris, use O_CREAT although it should not be necessary.

diff --git a/lib/supersede.c b/lib/supersede.c
index 92317f2..a03cc6d 100644
--- a/lib/supersede.c
+++ b/lib/supersede.c
@@ -78,6 +78,16 @@ open_supersede (const char *filename, int flags, mode_t mode,
                 struct supersede_final_action *action)
 {
   int fd;
+  /* Extra flags for existing devices.  */
+  int extra_flags =
+    #if defined __sun
+    /* open ("/dev/null", O_TRUNC | O_WRONLY) fails with error EINVAL on Solaris
+       zones.  See <https://www.illumos.org/issues/13035>.  As a workaround, add
+       the O_CREAT flag, although it ought not to be necessary.  */
+    O_CREAT;
+    #else
+    0;
+    #endif
 
   if (supersede_if_exists)
     {
@@ -89,7 +99,7 @@ open_supersede (const char *filename, int flags, mode_t mode,
               && ! S_ISREG (statbuf.st_mode)
               /* The file exists and is possibly a character device, socket, or
                  something like that.  */
-              && ((fd = open (filename, flags, mode)) >= 0
+              && ((fd = open (filename, flags | extra_flags, mode)) >= 0
                   || errno != ENOENT))
             {
               if (fd >= 0)
@@ -167,7 +177,7 @@ open_supersede (const char *filename, int flags, mode_t mode,
                         {
                           /* It is possibly a character device, socket, or
                              something like that.  */
-                          fd = open (canon_filename, flags, mode);
+                          fd = open (canon_filename, flags | extra_flags, mode);
                           if (fd >= 0)
                             {
                               free (canon_filename);
@@ -197,6 +207,28 @@ open_supersede (const char *filename, int flags, mode_t mode,
               action->final_rename_temp = NULL;
               action->final_rename_dest = NULL;
             }
+          #if defined __sun
+          /* Work around <https://www.illumos.org/issues/13035>.  */
+          else if (errno == EINVAL)
+            {
+              struct stat statbuf;
+
+              if (stat (filename, &statbuf) >= 0
+                  && ! S_ISREG (statbuf.st_mode))
+                {
+                  /* The file exists and is possibly a character device, socket,
+                     or something like that.  As a workaround, add the O_CREAT
+                     flag, although it ought not to be necessary.*/
+                  fd = open (filename, flags | extra_flags, mode);
+                  if (fd >= 0)
+                    {
+                      /* The file exists.  */
+                      action->final_rename_temp = NULL;
+                      action->final_rename_dest = NULL;
+                    }
+                }
+            }
+          #endif
           else if (errno == ENOENT)
             {
               /* The file does not exist.  Use a temporary file.  */



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: supersede: Avoid a failure when writing to /dev/null in Solaris zones
  2020-08-23 17:38 supersede: Avoid a failure when writing to /dev/null in Solaris zones Bruno Haible
@ 2020-09-20 19:31 ` Bruno Haible
  0 siblings, 0 replies; 3+ messages in thread
From: Bruno Haible @ 2020-09-20 19:31 UTC (permalink / raw)
  To: bug-gnulib

On 2020-08-23 I did:
> 2020-08-23  Bruno Haible  <bruno@clisp.org>
> 
> 	supersede: Avoid a failure when writing to /dev/null in Solaris zones.
> 	Reported by Jörg Sonnenberger <joerg@netbsd.org>
> 	via Thomas Klausner <tk@giga.or.at> in
> 	<https://pkgsrc.se/files.php?messageId=20200812233110.30230FB28@cvs.NetBSD.org>.
> 	* lib/supersede.c (open_supersede): When opening an existing non-regular
> 	file on Solaris, use O_CREAT although it should not be necessary.

The same workaround is also needed on native Windows (mingw and MSVC).


2020-09-20  Bruno Haible  <bruno@clisp.org>

	supersede: Fix test failures on native Windows.
	* lib/supersede.c (open_supersede): Handle non-regular files on native
	Windows like on Solaris.
	* tests/test-supersede-open.h (test_open_supersede): Use O_BINARY flag.

diff --git a/lib/supersede.c b/lib/supersede.c
index a03cc6d..a3dfa4f 100644
--- a/lib/supersede.c
+++ b/lib/supersede.c
@@ -80,10 +80,12 @@ open_supersede (const char *filename, int flags, mode_t mode,
   int fd;
   /* Extra flags for existing devices.  */
   int extra_flags =
-    #if defined __sun
+    #if defined __sun || (defined _WIN32 && !defined __CYGWIN__)
     /* open ("/dev/null", O_TRUNC | O_WRONLY) fails with error EINVAL on Solaris
-       zones.  See <https://www.illumos.org/issues/13035>.  As a workaround, add
-       the O_CREAT flag, although it ought not to be necessary.  */
+       zones.  See <https://www.illumos.org/issues/13035>.
+       Likewise for open ("NUL", O_TRUNC | O_RDWR) on native Windows.
+       As a workaround, add the O_CREAT flag, although it ought not to be
+       necessary.  */
     O_CREAT;
     #else
     0;
@@ -207,8 +209,8 @@ open_supersede (const char *filename, int flags, mode_t mode,
               action->final_rename_temp = NULL;
               action->final_rename_dest = NULL;
             }
-          #if defined __sun
-          /* Work around <https://www.illumos.org/issues/13035>.  */
+          #if defined __sun || (defined _WIN32 && !defined __CYGWIN__)
+          /* See the comment regarding extra_flags, above.  */
           else if (errno == EINVAL)
             {
               struct stat statbuf;
diff --git a/tests/test-supersede-open.h b/tests/test-supersede-open.h
index f3b9b15..74a56d8 100644
--- a/tests/test-supersede-open.h
+++ b/tests/test-supersede-open.h
@@ -30,7 +30,7 @@ test_open_supersede (bool supersede_if_exists, bool supersede_if_does_not_exist)
     ASSERT (stat (filename, &statbuf) < 0);
 
     struct supersede_final_action action;
-    int fd = open_supersede (filename, O_RDWR | O_TRUNC, 0666,
+    int fd = open_supersede (filename, O_RDWR | O_BINARY | O_TRUNC, 0666,
                              supersede_if_exists, supersede_if_does_not_exist,
                              &action);
     ASSERT (fd >= 0);
@@ -56,7 +56,7 @@ test_open_supersede (bool supersede_if_exists, bool supersede_if_does_not_exist)
     ino_t orig_ino = statbuf.st_ino;
 
     struct supersede_final_action action;
-    int fd = open_supersede (filename, O_RDWR | O_TRUNC, 0666,
+    int fd = open_supersede (filename, O_RDWR | O_BINARY | O_TRUNC, 0666,
                              supersede_if_exists, supersede_if_does_not_exist,
                              &action);
     ASSERT (fd >= 0);
@@ -101,7 +101,7 @@ test_open_supersede (bool supersede_if_exists, bool supersede_if_does_not_exist)
     ASSERT (stat (DEV_NULL, &statbuf) == 0);
 
     struct supersede_final_action action;
-    int fd = open_supersede (DEV_NULL, O_RDWR | O_TRUNC, 0666,
+    int fd = open_supersede (DEV_NULL, O_RDWR | O_BINARY | O_TRUNC, 0666,
                              supersede_if_exists, supersede_if_does_not_exist,
                              &action);
     ASSERT (fd >= 0);
@@ -125,7 +125,7 @@ test_open_supersede (bool supersede_if_exists, bool supersede_if_does_not_exist)
 
         struct supersede_final_action action;
         int fd =
-          open_supersede (linkname, O_RDWR | O_TRUNC, 0666,
+          open_supersede (linkname, O_RDWR | O_BINARY | O_TRUNC, 0666,
                           supersede_if_exists, supersede_if_does_not_exist,
                           &action);
         ASSERT (fd >= 0);
@@ -180,7 +180,7 @@ test_open_supersede (bool supersede_if_exists, bool supersede_if_does_not_exist)
 
         struct supersede_final_action action;
         int fd =
-          open_supersede (linkname, O_RDWR | O_TRUNC, 0666,
+          open_supersede (linkname, O_RDWR | O_BINARY | O_TRUNC, 0666,
                           supersede_if_exists, supersede_if_does_not_exist,
                           &action);
         ASSERT (fd >= 0);
@@ -209,7 +209,7 @@ test_open_supersede (bool supersede_if_exists, bool supersede_if_does_not_exist)
 
         struct supersede_final_action action;
         int fd =
-          open_supersede (linkname, O_RDWR | O_TRUNC, 0666,
+          open_supersede (linkname, O_RDWR | O_BINARY | O_TRUNC, 0666,
                           supersede_if_exists, supersede_if_does_not_exist,
                           &action);
         ASSERT (fd >= 0);
@@ -243,7 +243,7 @@ test_open_supersede (bool supersede_if_exists, bool supersede_if_does_not_exist)
 
         struct supersede_final_action action;
         int fd =
-          open_supersede (linkname, O_RDWR | O_TRUNC, 0666,
+          open_supersede (linkname, O_RDWR | O_BINARY | O_TRUNC, 0666,
                           supersede_if_exists, supersede_if_does_not_exist,
                           &action);
         ASSERT (fd < 0);



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* supersede: Avoid a failure when writing to /dev/null in Solaris zones
@ 2022-09-12  8:51 Bruno Haible
  0 siblings, 0 replies; 3+ messages in thread
From: Bruno Haible @ 2022-09-12  8:51 UTC (permalink / raw)
  To: bug-gnulib

On Solaris 11.3 (gcc211.fsffrance.org), I'm seeing a test-supersede failure.
It's much like the one seen on Solaris/Illumos and fixed through
<https://lists.gnu.org/archive/html/bug-gnulib/2020-08/msg00206.html>.
The only difference is that here, the open() call fails with error EACCES,
not EINVAL.

This patch fixes the test failure.


2022-09-12  Bruno Haible  <bruno@clisp.org>

	supersede: Avoid a failure when writing to /dev/null in Solaris zones.
	* lib/supersede.c (open_supersede): Treat EACCES (seen on Solaris 11.3)
	like EINVAL (seen on Illumos).

diff --git a/lib/supersede.c b/lib/supersede.c
index a72c4f40e8..7fbd619fde 100644
--- a/lib/supersede.c
+++ b/lib/supersede.c
@@ -83,9 +83,12 @@ open_supersede (const char *filename, int flags, mode_t mode,
   /* Extra flags for existing devices.  */
   int extra_flags =
     #if defined __sun || (defined _WIN32 && !defined __CYGWIN__)
-    /* open ("/dev/null", O_TRUNC | O_WRONLY) fails with error EINVAL on Solaris
-       zones.  See <https://www.illumos.org/issues/13035>.
-       Likewise for open ("NUL", O_TRUNC | O_RDWR) on native Windows.
+    /* open ("/dev/null", O_TRUNC | O_WRONLY) fails on Solaris zones:
+         - with error EINVAL on Illumos, see
+           <https://www.illumos.org/issues/13035>,
+         - with error EACCES on Solaris 11.3.
+       Likewise, open ("NUL", O_TRUNC | O_RDWR) fails with error EINVAL on
+       native Windows.
        As a workaround, add the O_CREAT flag, although it ought not to be
        necessary.  */
     O_CREAT;
@@ -204,7 +207,7 @@ open_supersede (const char *filename, int flags, mode_t mode,
             }
           #if defined __sun || (defined _WIN32 && !defined __CYGWIN__)
           /* See the comment regarding extra_flags, above.  */
-          else if (errno == EINVAL)
+          else if (errno == EINVAL || errno == EACCES)
             {
               struct stat statbuf;
 





^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-09-12  8:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-23 17:38 supersede: Avoid a failure when writing to /dev/null in Solaris zones Bruno Haible
2020-09-20 19:31 ` Bruno Haible
  -- strict thread matches above, loose matches on Subject: below --
2022-09-12  8:51 Bruno Haible

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).