On 12/08/14 13:31, Waldemar Brodkorb wrote:
Hi Anthony,

I tried your patch, but I have a minor issue when building the added
tests on noMMU (like Blackfin):

Thanks. This is just a test but let me look at this wrt to noMMU and see what can be done. You can probably avoid the fork but you need the exec because that's the whole point of O_CLOEXEC.



/home/wbx/embedded-test/openadk/toolchain_toolchain-bfin_uclibc-ng_bfin/usr/bin/bfin-openadk-linux-uclibc-gcc
-nostdinc -I../../install_dir/usr/include -I../../test -D_GNU_SOURCE
-I/home/wbx/embedded-test/openadk/target_toolchain-bfin_uclibc-ng_bfin/usr/include/
-isystem
/home/wbx/embedded-test/openadk/toolchain_toolchain-bfin_uclibc-ng_bfin/usr/lib/gcc/bfin-openadk-linux-uclibc/4.5.4/include-fixed
-isystem
/home/wbx/embedded-test/openadk/toolchain_toolchain-bfin_uclibc-ng_bfin/usr/lib/gcc/bfin-openadk-linux-uclibc/4.5.4/include
-Os -fstrict-aliasing -mfdpic -Wall -Wstrict-prototypes
-Wstrict-aliasing -Wstrict-prototypes    -c
test-mkostemp-O_CLOEXEC.c -o test-mkostemp-O_CLOEXEC.o
test-mkostemp-O_CLOEXEC.c: In function 'main':
test-mkostemp-O_CLOEXEC.c:21:5: warning: implicit declaration of
function 'fork'
/home/wbx/embedded-test/openadk/toolchain_toolchain-bfin_uclibc-ng_bfin/usr/bin/bfin-openadk-linux-uclibc-gcc
-Wl,-EL -Wl,-melf32bfinfd -Wl,-z,now -Wl,-s
-Wl,-rpath,/home/wbx/embedded-test/openadk/toolchain_build_toolchain-bfin_uclibc-ng_bfin/w-uClibc-ng-1.0.0-1/uClibc-ng-1.0.0/test/stdlib
-Wl,--dynamic-linker,/lib//ld-uClibc.so.1 test-mkostemp-O_CLOEXEC.o
-o test-mkostemp-O_CLOEXEC
test-mkostemp-O_CLOEXEC.o: In function `_main':
test-mkostemp-O_CLOEXEC.c:(.text+0x46): undefined reference to
`_fork'
collect2: ld returned 1 exit status
make[7]: *** [test-mkostemp-O_CLOEXEC] Error 1

Avoid the test on noMMU or is it possible to not directly use
fork()?

best regards
  Waldemar

Anthony G. Basile wrote,

On 10/27/14 16:13, bas...@opensource.dyc.edu wrote:
From: "Anthony G. Basile" <bluen...@gentoo.org>

mkostemp(char *template, int flags) generates a unique temporary
filename from a template.  The flags parameter accepts three of
the same flags as open(2): O_APPEND, O_CLOEXEC, and O_SYNC.  The
current implementation of mkostemp(3) does not respect the flags
and in fact confuses the flags with the file mode which should
always be S_IRUSR | S_IWUSR.  This patch corrects this issue.


Can someone please review this.  Thanks.



Signed-off-by: Anthony G. Basile <bluen...@gentoo.org>
---
  libc/inet/getaddrinfo.c               |  2 +-
  libc/misc/internals/tempname.c        |  6 +++---
  libc/misc/internals/tempname.h        |  2 +-
  libc/stdio/tempnam.c                  |  2 +-
  libc/stdio/tmpfile.c                  |  2 +-
  libc/stdio/tmpnam.c                   |  2 +-
  libc/stdio/tmpnam_r.c                 |  2 +-
  libc/stdlib/mkdtemp.c                 |  2 +-
  libc/stdlib/mkostemp.c                |  4 +++-
  libc/stdlib/mkostemp64.c              |  2 +-
  libc/stdlib/mkstemp.c                 |  2 +-
  libc/stdlib/mkstemp64.c               |  2 +-
  libc/stdlib/mktemp.c                  |  2 +-
  libpthread/nptl/sem_open.c            |  2 +-
  test/.gitignore                       |  2 ++
  test/stdlib/test-mkostemp-O_CLOEXEC.c | 40 +++++++++++++++++++++++++++++++++++
  test/stdlib/test-mkostemp-child.c     | 22 +++++++++++++++++++
  17 files changed, 82 insertions(+), 16 deletions(-)
  create mode 100644 test/stdlib/test-mkostemp-O_CLOEXEC.c
  create mode 100644 test/stdlib/test-mkostemp-child.c

diff --git a/libc/inet/getaddrinfo.c b/libc/inet/getaddrinfo.c
index b61d69c..168adb1 100644
--- a/libc/inet/getaddrinfo.c
+++ b/libc/inet/getaddrinfo.c
@@ -308,7 +308,7 @@ gaih_local(const char *name, const struct gaih_service 
*service,
                char *buf = ((struct sockaddr_un *)ai->ai_addr)->sun_path;

                if (__path_search(buf, L_tmpnam, NULL, NULL, 0) != 0
-                || __gen_tempname(buf, __GT_NOCREATE, 0) != 0
+                || __gen_tempname(buf, __GT_NOCREATE, 0, 0) != 0
                ) {
                        return -EAI_SYSTEM;
                }
diff --git a/libc/misc/internals/tempname.c b/libc/misc/internals/tempname.c
index 18fd823..edcc31c 100644
--- a/libc/misc/internals/tempname.c
+++ b/libc/misc/internals/tempname.c
@@ -177,7 +177,7 @@ static void brain_damaged_fillrand(unsigned char *buf, 
unsigned int len)
     __GT_DIR:            create a directory with given mode.

  */
-int attribute_hidden __gen_tempname (char *tmpl, int kind, mode_t mode)
+int attribute_hidden __gen_tempname (char *tmpl, int kind, int flags, mode_t 
mode)
  {
      char *XXXXXX;
      unsigned int i;
@@ -219,11 +219,11 @@ int attribute_hidden __gen_tempname (char *tmpl, int 
kind, mode_t mode)
                        fd = 0;
                }
            case __GT_FILE:
-               fd = open (tmpl, O_RDWR | O_CREAT | O_EXCL, mode);
+               fd = open (tmpl, O_RDWR | O_CREAT | O_EXCL | flags, mode);
                break;
  #if defined __UCLIBC_HAS_LFS__
            case __GT_BIGFILE:
-               fd = open64 (tmpl, O_RDWR | O_CREAT | O_EXCL, mode);
+               fd = open64 (tmpl, O_RDWR | O_CREAT | O_EXCL | flags, mode);
                break;
  #endif
            case __GT_DIR:
diff --git a/libc/misc/internals/tempname.h b/libc/misc/internals/tempname.h
index e75b632..edfe26d 100644
--- a/libc/misc/internals/tempname.h
+++ b/libc/misc/internals/tempname.h
@@ -10,7 +10,7 @@ extern int ___path_search (char *tmpl, size_t tmpl_len, const 
char *dir,
                const char *pfx /*, int try_tmpdir */) attribute_hidden;
  #define __path_search(tmpl, tmpl_len, dir, pfx, try_tmpdir) 
___path_search(tmpl, tmpl_len, dir, pfx)

-extern int __gen_tempname (char *__tmpl, int __kind, mode_t mode) 
attribute_hidden;
+extern int __gen_tempname (char *__tmpl, int __kind, int flags, mode_t mode) 
attribute_hidden;

  /* The __kind argument to __gen_tempname may be one of: */
  #define __GT_FILE     0       /* create a file */
diff --git a/libc/stdio/tempnam.c b/libc/stdio/tempnam.c
index 232ed02..74bb26e 100644
--- a/libc/stdio/tempnam.c
+++ b/libc/stdio/tempnam.c
@@ -35,7 +35,7 @@ tempnam (const char *dir, const char *pfx)
    if (__path_search (buf, FILENAME_MAX, dir, pfx, 1))
      return NULL;

-  if (__gen_tempname (buf, __GT_NOCREATE, 0))
+  if (__gen_tempname (buf, __GT_NOCREATE, 0, 0))
      return NULL;

    return strdup (buf);
diff --git a/libc/stdio/tmpfile.c b/libc/stdio/tmpfile.c
index a9bf474..83c85b5 100644
--- a/libc/stdio/tmpfile.c
+++ b/libc/stdio/tmpfile.c
@@ -35,7 +35,7 @@ FILE * tmpfile (void)

      if (__path_search (buf, FILENAME_MAX, NULL, "tmpf", 0))
        return NULL;
-    fd = __gen_tempname (buf, __GT_FILE, S_IRUSR | S_IWUSR);
+    fd = __gen_tempname (buf, __GT_FILE, 0, S_IRUSR | S_IWUSR);
      if (fd < 0)
        return NULL;

diff --git a/libc/stdio/tmpnam.c b/libc/stdio/tmpnam.c
index 88b9bff..ffed862 100644
--- a/libc/stdio/tmpnam.c
+++ b/libc/stdio/tmpnam.c
@@ -40,7 +40,7 @@ tmpnam (char *s)
                        0))
      return NULL;

-  if (__builtin_expect (__gen_tempname (tmpbuf, __GT_NOCREATE, 0), 0))
+  if (__builtin_expect (__gen_tempname (tmpbuf, __GT_NOCREATE, 0, 0), 0))
      return NULL;

    if (s == NULL)
diff --git a/libc/stdio/tmpnam_r.c b/libc/stdio/tmpnam_r.c
index 8cde84d..bfd60a4 100644
--- a/libc/stdio/tmpnam_r.c
+++ b/libc/stdio/tmpnam_r.c
@@ -27,7 +27,7 @@ char * tmpnam_r (char *s)

      if (__path_search (s, L_tmpnam, NULL, NULL, 0))
        return NULL;
-    if (__gen_tempname (s, __GT_NOCREATE, 0))
+    if (__gen_tempname (s, __GT_NOCREATE, 0, 0))
        return NULL;

      return s;
diff --git a/libc/stdlib/mkdtemp.c b/libc/stdlib/mkdtemp.c
index da7598a..e6d4a36 100644
--- a/libc/stdlib/mkdtemp.c
+++ b/libc/stdlib/mkdtemp.c
@@ -29,7 +29,7 @@
     (This function comes from OpenBSD.) */
  char * mkdtemp (char *template)
  {
-  if (__gen_tempname (template, __GT_DIR, S_IRUSR | S_IWUSR | S_IXUSR))
+  if (__gen_tempname (template, __GT_DIR, 0, S_IRUSR | S_IWUSR | S_IXUSR))
      return NULL;
    else
      return template;
diff --git a/libc/stdlib/mkostemp.c b/libc/stdlib/mkostemp.c
index 0369235..912be30 100644
--- a/libc/stdlib/mkostemp.c
+++ b/libc/stdlib/mkostemp.c
@@ -17,6 +17,7 @@

  #include <stdio.h>
  #include <stdlib.h>
+#include <fcntl.h>
  #include "../misc/internals/tempname.h"

  /* Generate a unique temporary file name from TEMPLATE.
@@ -26,5 +27,6 @@
  int
  mkostemp (char *template, int flags)
  {
-  return __gen_tempname (template, __GT_FILE, flags);
+  flags -= flags & O_ACCMODE; /* Remove O_RDONLY, O_WRONLY, and O_RDWR. */
+  return __gen_tempname (template, __GT_FILE, flags, S_IRUSR | S_IWUSR);
  }
diff --git a/libc/stdlib/mkostemp64.c b/libc/stdlib/mkostemp64.c
index d21def5..c6d6d84 100644
--- a/libc/stdlib/mkostemp64.c
+++ b/libc/stdlib/mkostemp64.c
@@ -27,5 +27,5 @@
  int
  mkostemp64 (char *template, int flags)
  {
-  return __gen_tempname (template, __GT_BIGFILE, flags | O_LARGEFILE);
+  return __gen_tempname (template, __GT_BIGFILE, flags | O_LARGEFILE, S_IRUSR 
| S_IWUSR | S_IXUSR);
  }
diff --git a/libc/stdlib/mkstemp.c b/libc/stdlib/mkstemp.c
index 61c7175..a3a1595 100644
--- a/libc/stdlib/mkstemp.c
+++ b/libc/stdlib/mkstemp.c
@@ -26,5 +26,5 @@
     Then open the file and return a fd. */
  int mkstemp (char *template)
  {
-    return __gen_tempname (template, __GT_FILE, S_IRUSR | S_IWUSR);
+    return __gen_tempname (template, __GT_FILE, 0, S_IRUSR | S_IWUSR);
  }
diff --git a/libc/stdlib/mkstemp64.c b/libc/stdlib/mkstemp64.c
index e29be2d..6f2ee3e 100644
--- a/libc/stdlib/mkstemp64.c
+++ b/libc/stdlib/mkstemp64.c
@@ -26,5 +26,5 @@
     Then open the file and return a fd. */
  int mkstemp64 (char *template)
  {
-    return __gen_tempname (template, __GT_BIGFILE, S_IRUSR | S_IWUSR);
+    return __gen_tempname (template, __GT_BIGFILE, 0, S_IRUSR | S_IWUSR);
  }
diff --git a/libc/stdlib/mktemp.c b/libc/stdlib/mktemp.c
index edd001d..1ff93da 100644
--- a/libc/stdlib/mktemp.c
+++ b/libc/stdlib/mktemp.c
@@ -24,7 +24,7 @@
   * they are replaced with a string that makes the filename unique.  */
  char *mktemp(char *template)
  {
-       if (__gen_tempname (template, __GT_NOCREATE, 0) < 0)
+       if (__gen_tempname (template, __GT_NOCREATE, 0, 0) < 0)
                /* We return the null string if we can't find a unique file 
name.  */
                template[0] = '\0';

diff --git a/libpthread/nptl/sem_open.c b/libpthread/nptl/sem_open.c
index 1b36164..3a72079 100644
--- a/libpthread/nptl/sem_open.c
+++ b/libpthread/nptl/sem_open.c
@@ -336,7 +336,7 @@ sem_open (const char *name, int oflag, ...)
        mempcpy (mempcpy (tmpfname, mountpoint.dir, mountpoint.dirlen),
        "XXXXXX", 7);

-      fd = __gen_tempname (tmpfname, __GT_FILE, mode);
+      fd = __gen_tempname (tmpfname, __GT_FILE, 0, mode);
        if (fd == -1)
          return SEM_FAILED;

diff --git a/test/.gitignore b/test/.gitignore
index 8f32031..5944f0a 100644
--- a/test/.gitignore
+++ b/test/.gitignore
@@ -274,6 +274,8 @@ stdlib/testarc4random
  stdlib/testatexit
  stdlib/test-canon
  stdlib/test-canon2
+stdlib/test-mkostemp-O_CLOEXEC
+stdlib/test-mkostemp-child
  stdlib/teston_exit
  stdlib/teststrtol
  stdlib/teststrtoq
diff --git a/test/stdlib/test-mkostemp-O_CLOEXEC.c 
b/test/stdlib/test-mkostemp-O_CLOEXEC.c
new file mode 100644
index 0000000..5652086
--- /dev/null
+++ b/test/stdlib/test-mkostemp-O_CLOEXEC.c
@@ -0,0 +1,40 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <sys/wait.h>
+#include <errno.h>
+
+int main(int argc, char *argv[]) {
+    int fd, status;
+    char buff[5];
+    char template[] = "/tmp/test-mkostemp.XXXXXX";
+
+    fd = mkostemp(template, O_CLOEXEC);
+    unlink(template);
+
+    snprintf(buff, 5, "%d", fd);
+
+    if(!fork())
+        if(execl("./test-mkostemp-child", "test-mkostemp-child", buff, NULL) 
== -1)
+            exit(EXIT_FAILURE);
+
+    wait(&status);
+
+    memset(buff, 0, 5);
+    lseek(fd, 0, SEEK_SET);
+    errno = 0;
+    if(read(fd, buff, 5) == -1)
+        exit(EXIT_FAILURE);
+
+    if(!strncmp(buff, "test", 5))
+        exit(EXIT_FAILURE);
+    else
+        exit(EXIT_SUCCESS);
+
+    close(fd);
+    exit(EXIT_SUCCESS);
+}
diff --git a/test/stdlib/test-mkostemp-child.c 
b/test/stdlib/test-mkostemp-child.c
new file mode 100644
index 0000000..708bd80
--- /dev/null
+++ b/test/stdlib/test-mkostemp-child.c
@@ -0,0 +1,22 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+int main(int argc, char *argv[]) {
+    int fd;
+
+    /* This file gets built and run as a test, but its
+     * really just a helper for test-mkostemp-O_CLOEXEC.c.
+     * So, we'll always return succcess.
+     */
+    if(argc != 2)
+        exit(EXIT_SUCCESS);
+
+    sscanf(argv[1], "%d", &fd);
+
+    if(write(fd, "test\0", 5) == -1)
+        ; /* Don't Panic!  Failure is okay here. */
+
+    close(fd);
+    exit(EXIT_SUCCESS);
+}



--
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
(716) 829-8197
_______________________________________________
uClibc mailing list
uClibc@uclibc.org
http://lists.busybox.net/mailman/listinfo/uclibc



--
Anthony G. Basile, Ph. D.
Chair of Information Technology
D'Youville College
Buffalo, NY 14201
(716) 829-8197
_______________________________________________
uClibc mailing list
uClibc@uclibc.org
http://lists.busybox.net/mailman/listinfo/uclibc

Reply via email to