| Message ID | 1414440814-5403-1-git-send-email-basile@opensource.dyc.edu |
|---|---|
| State | Accepted |
| Commit | 638a23483b40c5b606ee323e6612e7e454e5154b |
| Headers | show |
On 10/27/14 16:13, basile@opensource.dyc.edu wrote: > From: "Anthony G. Basile" <blueness@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@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); > +} >
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@opensource.dyc.edu wrote: > >From: "Anthony G. Basile" <blueness@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@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 >
Hi Anthony, Anthony G. Basile wrote, > 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. If you believe the test make no sense with noMMU, I am happy if you just disable it for noMMU targets. best regards Waldemar
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, basile@opensource.dyc.edu wrote: >>> From: "Anthony G. Basile" <blueness@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@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 >>
On Tue, Dec 09, 2014 at 09:13:54AM -0500, Anthony G. Basile wrote: > 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. If uclibc has posix_spawn yet you could use that instead for the test. Also vfork+exec would be viable. Rich
On 12/09/14 09:13, Anthony G. Basile wrote: > 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. > > Hey look, its magically fixed in the latest commit: diff --git a/test/stdlib/test-mkostemp-O_CLOEXEC.c b/test/stdlib/test-mkostemp-O_CLOEXEC.c +#if !defined __ARCH_USE_MMU__ +# define fork vfork +#endif @Bernhard. Thanks :)
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); +}