Re: fix mkostemp, add pipe2-safer

[Eric Blake]

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

According to Bruno Haible on 12/6/2009 5:18 PM:
>   - The code accesses an undefined variable 'result'.

You caught one, but missed the other.  I noticed some other things while
fixing it.  Pushing these:

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

iEYEARECAAYFAksd390ACgkQ84KuGfSFAYCczwCgob08TPrWRDlidwiZsXokK6Fj
DFkAn2Ygm8In0/BAn+fGkHnrpEjTjWIM
=yWmr
-----END PGP SIGNATURE-----

From a1afba1b2122388e9a120b6a1f011367e6db1518 Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 7 Dec 2009 10:17:07 -0700
Subject: [PATCH 1/2] unistd-safer: add unit test

Add more unit tests.  Meanwhile, fix compilation error on mingw when
testing unistd-safer and fchdir together; and avoid gcc warning on
platforms without setmode.

* modules/unistd-safer-tests: New file.
* tests/test-dup-safer.c: Likewise.
* tests/test-cloexec.c (setmode): Avoid compiler warning.
* tests/test-dup2.c (setmode): Likewise.
* lib/cloexec.c (dup_cloexec): Fix mingw compile error.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog                  |    7 ++
 lib/cloexec.c              |    2 +-
 modules/unistd-safer-tests |   13 +++
 tests/test-cloexec.c       |    3 +-
 tests/test-dup-safer.c     |  186 ++++++++++++++++++++++++++++++++++++++++++++
 tests/test-dup2.c          |    3 +-
 6 files changed, 211 insertions(+), 3 deletions(-)
 create mode 100644 modules/unistd-safer-tests
 create mode 100644 tests/test-dup-safer.c

diff --git a/ChangeLog b/ChangeLog
index 3cf1bdb..73e6faa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
 2009-12-07  Eric Blake  <address@hidden>

+       unistd-safer: add unit test
+       * modules/unistd-safer-tests: New file.
+       * tests/test-dup-safer.c: Likewise.
+       * tests/test-cloexec.c (setmode): Avoid compiler warning.
+       * tests/test-dup2.c (setmode): Likewise.
+       * lib/cloexec.c (dup_cloexec): Fix mingw compile error.
+
        cloexec: preserve text vs. binary across dup_cloexec
        * lib/cloexec.c (dup_cloexec) [W32]: Query and use translation
        mode.
diff --git a/lib/cloexec.c b/lib/cloexec.c
index 57e0be5..18985cb 100644
--- a/lib/cloexec.c
+++ b/lib/cloexec.c
@@ -128,7 +128,7 @@ dup_cloexec (int fd)

 #  if REPLACE_FCHDIR
   if (0 <= nfd)
-    result = _gl_register_dup (fd, nfd);
+    nfd = _gl_register_dup (fd, nfd);
 #  endif
   return nfd;

diff --git a/modules/unistd-safer-tests b/modules/unistd-safer-tests
new file mode 100644
index 0000000..17b17a0
--- /dev/null
+++ b/modules/unistd-safer-tests
@@ -0,0 +1,13 @@
+Files:
+tests/test-dup-safer.c
+
+Depends-on:
+binary-io
+cloexec
+stdbool
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-dup-safer
+check_PROGRAMS += test-dup-safer
diff --git a/tests/test-cloexec.c b/tests/test-cloexec.c
index a29d1be..1f43014 100644
--- a/tests/test-cloexec.c
+++ b/tests/test-cloexec.c
@@ -69,7 +69,8 @@ is_inheritable (int fd)
 }

 #if !O_BINARY
-# define setmode(f,m) 0
+# define setmode(f,m) zero ()
+static int zero (void) { return 0; }
 #endif

 /* Return non-zero if FD is open in the given MODE, which is either
diff --git a/tests/test-dup-safer.c b/tests/test-dup-safer.c
new file mode 100644
index 0000000..24cc9e5
--- /dev/null
+++ b/tests/test-dup-safer.c
@@ -0,0 +1,186 @@
+/* Test that dup_safer leaves standard fds alone.
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Eric Blake <address@hidden>, 2009.  */
+
+#include <config.h>
+
+#include "unistd--.h"
+
+#include <fcntl.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+
+#include "binary-io.h"
+#include "cloexec.h"
+
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+/* Get declarations of the Win32 API functions.  */
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>
+#endif
+
+#if !O_BINARY
+# define setmode(f,m) zero ()
+static int zero (void) { return 0; }
+#endif
+#ifndef O_CLOEXEC
+# define O_CLOEXEC 0
+#endif
+
+/* This test intentionally closes stderr.  So, we arrange to have fd 10
+   (outside the range of interesting fd's during the test) set up to
+   duplicate the original stderr.  */
+
+#define BACKUP_STDERR_FILENO 10
+static FILE *myerr;
+
+#define ASSERT(expr) \
+  do                                                                         \
+    {                                                                        \
+      if (!(expr))                                                           \
+        {                                                                    \
+          fprintf (myerr, "%s:%d: assertion failed\n", __FILE__, __LINE__);  \
+          fflush (myerr);                                                    \
+          abort ();                                                          \
+        }                                                                    \
+    }                                                                        \
+  while (0)
+
+/* Return true if FD is open.  */
+static bool
+is_open (int fd)
+{
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+  /* On Win32, the initial state of unassigned standard file
+     descriptors is that they are open but point to an
+     INVALID_HANDLE_VALUE, and there is no fcntl.  */
+  return (HANDLE) _get_osfhandle (fd) != INVALID_HANDLE_VALUE;
+#else
+# ifndef F_GETFL
+#  error Please port fcntl to your platform
+# endif
+  return 0 <= fcntl (fd, F_GETFL);
+#endif
+}
+
+/* Return true if FD is open and inheritable across exec/spawn.  */
+static bool
+is_inheritable (int fd)
+{
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+  /* On Win32, the initial state of unassigned standard file
+     descriptors is that they are open but point to an
+     INVALID_HANDLE_VALUE, and there is no fcntl.  */
+  HANDLE h = (HANDLE) _get_osfhandle (fd);
+  DWORD flags;
+  if (h == INVALID_HANDLE_VALUE || GetHandleInformation (h, &flags) == 0)
+    return 0;
+  return (flags & HANDLE_FLAG_INHERIT) != 0;
+#else
+# ifndef F_GETFD
+#  error Please port fcntl to your platform
+# endif
+  int i = fcntl (fd, F_GETFD);
+  return 0 <= i && (i & FD_CLOEXEC) == 0;
+#endif
+}
+
+/* Return true if FD is open in the given MODE, which is either
+   O_TEXT or O_BINARY.  */
+static bool
+is_mode (int fd, int mode)
+{
+  int value = setmode (fd, O_BINARY);
+  setmode (fd, value);
+  return mode == value;
+}
+
+#define witness "test-dup-safer.txt"
+
+int
+main (void)
+{
+  int i;
+  int fd;
+
+  /* We close fd 2 later, so save it in fd 10.  */
+  if (dup2 (STDERR_FILENO, BACKUP_STDERR_FILENO) != BACKUP_STDERR_FILENO
+      || (myerr = fdopen (BACKUP_STDERR_FILENO, "w")) == NULL)
+    return 2;
+
+  /* Create file for later checks.  */
+  fd = creat (witness, 0600);
+  ASSERT (STDERR_FILENO < fd);
+
+  /* Four iterations, with progressively more standard descriptors
+     closed.  */
+  for (i = -1; i <= STDERR_FILENO; i++)
+    {
+      if (0 <= i)
+        ASSERT (close (i) == 0);
+
+      /* Detect errors.  */
+      errno = 0;
+      ASSERT (dup (-1) == -1);
+      ASSERT (errno == EBADF);
+      errno = 0;
+      ASSERT (dup (10000000) == -1);
+      ASSERT (errno == EBADF);
+      close (fd + 1);
+      errno = 0;
+      ASSERT (dup (fd + 1) == -1);
+      ASSERT (errno == EBADF);
+
+      /* Preserve text vs. binary.  */
+      setmode (fd, O_BINARY);
+      ASSERT (dup (fd) == fd + 1);
+      ASSERT (is_open (fd + 1));
+      ASSERT (is_inheritable (fd + 1));
+      ASSERT (is_mode (fd + 1, O_BINARY));
+
+      ASSERT (close (fd + 1) == 0);
+      setmode (fd, O_TEXT);
+      ASSERT (dup (fd) == fd + 1);
+      ASSERT (is_open (fd + 1));
+      ASSERT (is_inheritable (fd + 1));
+      ASSERT (is_mode (fd + 1, O_TEXT));
+
+      /* Create cloexec copy.  */
+      ASSERT (close (fd + 1) == 0);
+      ASSERT (fd_safer_flag (dup_cloexec (fd), O_CLOEXEC) == fd + 1);
+      ASSERT (set_cloexec_flag (fd + 1, true) == 0);
+      ASSERT (is_open (fd + 1));
+      ASSERT (!is_inheritable (fd + 1));
+      ASSERT (close (fd) == 0);
+
+      /* dup always creates inheritable copies.  Also, check that
+         earliest slot past std fds is used.  */
+      ASSERT (dup (fd + 1) == fd);
+      ASSERT (is_open (fd));
+      ASSERT (is_inheritable (fd));
+      ASSERT (close (fd + 1) == 0);
+    }
+
+  /* Cleanup.  */
+  ASSERT (close (fd) == 0);
+  ASSERT (unlink (witness) == 0);
+
+  return 0;
+}
diff --git a/tests/test-dup2.c b/tests/test-dup2.c
index e6d3c62..b22d1e6 100644
--- a/tests/test-dup2.c
+++ b/tests/test-dup2.c
@@ -86,7 +86,8 @@ is_inheritable (int fd)
 }

 #if !O_BINARY
-# define setmode(f,m) 0
+# define setmode(f,m) zero ()
+static int zero (void) { return 0; }
 #endif

 /* Return non-zero if FD is open in the given MODE, which is either
-- 
1.6.5.rc1


From 67ac31ed5b8a602032d034bd91022ba329e6485f Mon Sep 17 00:00:00 2001
From: Eric Blake <address@hidden>
Date: Mon, 7 Dec 2009 17:15:07 -0700
Subject: [PATCH 2/2] unlink: fix m4 detection

The m4 test failed under -Werror due to implicit declaration.

* m4/unlink.m4 (gl_FUNC_UNLINK): Include correct header.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog    |    3 +++
 m4/unlink.m4 |    4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 73e6faa..7b78dca 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2009-12-07  Eric Blake  <address@hidden>

+       unlink: fix m4 detection
+       * m4/unlink.m4 (gl_FUNC_UNLINK): Include correct header.
+
        unistd-safer: add unit test
        * modules/unistd-safer-tests: New file.
        * tests/test-dup-safer.c: Likewise.
diff --git a/m4/unlink.m4 b/m4/unlink.m4
index bed2466..f89261c 100644
--- a/m4/unlink.m4
+++ b/m4/unlink.m4
@@ -1,4 +1,4 @@
-# unlink.m4 serial 2
+# unlink.m4 serial 3
 dnl Copyright (C) 2009 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -18,7 +18,7 @@ AC_DEFUN([gl_FUNC_UNLINK],
      fi
      AC_RUN_IFELSE(
        [AC_LANG_PROGRAM(
-         [[#include <stdio.h>
+         [[#include <unistd.h>
            #include <errno.h>
 ]], [[if (!unlink ("conftest.file/") || errno != ENOTDIR) return 1;
 #if HAVE_LSTAT
-- 
1.6.5.rc1