[PATCH] mkostemp: fix implementation

Waldemar Brodkorb wbx at openadk.org
Mon Dec 8 18:31:17 UTC 2014


Hi Anthony,

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

/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, basile at opensource.dyc.edu wrote:
> >From: "Anthony G. Basile" <blueness at 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 <blueness at 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 at uclibc.org
> http://lists.busybox.net/mailman/listinfo/uclibc
> 


More information about the uClibc mailing list