[PATCH 1/2] Extend __gen_tempname with mode argument

[Mikhail Gusarov-3]

sem_open(3) needs to create a temporary file in a way which can't
be efficiently implemented in terms of POSIX API. Extend
__gen_tempname with mode_t mode argument in order to ease
sem_open implementation.

Signed-off-by: Mikhail Gusarov <dottedmag@...>
---
 libc/inet/getaddrinfo.c        |    2 +-
 libc/misc/internals/tempname.c |   14 +++++++-------
 libc/misc/internals/tempname.h |    3 ++-
 libc/stdio/tempnam.c           |    2 +-
 libc/stdio/tmpfile.c           |    3 ++-
 libc/stdio/tmpnam.c            |    2 +-
 libc/stdio/tmpnam_r.c          |    2 +-
 libc/stdlib/mkdtemp.c          |    3 ++-
 libc/stdlib/mkstemp.c          |    3 ++-
 libc/stdlib/mkstemp64.c        |    3 ++-
 libc/stdlib/mktemp.c           |    2 +-
 11 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/libc/inet/getaddrinfo.c b/libc/inet/getaddrinfo.c
index b91486f..4a42b9e 100644
--- a/libc/inet/getaddrinfo.c
+++ b/libc/inet/getaddrinfo.c
@@ -307,7 +307,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
+ || __gen_tempname(buf, __GT_NOCREATE, 0) != 0
  ) {
  return -EAI_SYSTEM;
  }
diff --git a/libc/misc/internals/tempname.c b/libc/misc/internals/tempname.c
index cbd4ced..4abd3c6 100644
--- a/libc/misc/internals/tempname.c
+++ b/libc/misc/internals/tempname.c
@@ -168,14 +168,14 @@ static void brain_damaged_fillrand(unsigned char *buf, unsigned int len)
 
    KIND may be one of:
    __GT_NOCREATE:       simply verify that the name does not exist
-                        at the time of the call.
+                        at the time of the call. mode argument is ignored.
    __GT_FILE:           create the file using open(O_CREAT|O_EXCL)
-                        and return a read-write fd.  The file is mode 0600.
+                        and return a read-write fd with given mode.
    __GT_BIGFILE:        same as __GT_FILE but use open64().
-   __GT_DIR:            create a directory, which will be mode 0700.
+   __GT_DIR:            create a directory with given mode.
 
 */
-int attribute_hidden __gen_tempname (char *tmpl, int kind)
+int attribute_hidden __gen_tempname (char *tmpl, int kind, mode_t mode)
 {
     char *XXXXXX;
     unsigned int i;
@@ -217,15 +217,15 @@ int attribute_hidden __gen_tempname (char *tmpl, int kind)
  fd = 0;
  }
     case __GT_FILE:
- fd = open (tmpl, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR);
+ fd = open (tmpl, O_RDWR | O_CREAT | O_EXCL, mode);
  break;
 #if defined __UCLIBC_HAS_LFS__
     case __GT_BIGFILE:
- fd = open64 (tmpl, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR);
+ fd = open64 (tmpl, O_RDWR | O_CREAT | O_EXCL, mode);
  break;
 #endif
     case __GT_DIR:
- fd = mkdir (tmpl, S_IRUSR | S_IWUSR | S_IXUSR);
+ fd = mkdir (tmpl, mode);
  break;
     default:
  fd = -1;
diff --git a/libc/misc/internals/tempname.h b/libc/misc/internals/tempname.h
index ac40bef..e75b632 100644
--- a/libc/misc/internals/tempname.h
+++ b/libc/misc/internals/tempname.h
@@ -3,13 +3,14 @@
 
 #define __need_size_t
 #include <stddef.h>
+#include <sys/types.h>
 
 /* Disable support for $TMPDIR */
 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) attribute_hidden;
+extern int __gen_tempname (char *__tmpl, int __kind, 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 faf7c5e..66c905d 100644
--- a/libc/stdio/tempnam.c
+++ b/libc/stdio/tempnam.c
@@ -36,7 +36,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))
+  if (__gen_tempname (buf, __GT_NOCREATE, 0))
     return NULL;
 
   return strdup (buf);
diff --git a/libc/stdio/tmpfile.c b/libc/stdio/tmpfile.c
index 57f2527..c6b2dc8 100644
--- a/libc/stdio/tmpfile.c
+++ b/libc/stdio/tmpfile.c
@@ -18,6 +18,7 @@
 
 #include <features.h>
 #include <stdio.h>
+#include <sys/stat.h>
 #include <unistd.h>
 #include "../misc/internals/tempname.h"
 #include <not-cancel.h>
@@ -35,7 +36,7 @@ FILE * tmpfile (void)
 
     if (__path_search (buf, FILENAME_MAX, NULL, "tmpf", 0))
  return NULL;
-    fd = __gen_tempname (buf, __GT_FILE);
+    fd = __gen_tempname (buf, __GT_FILE, S_IRUSR | S_IWUSR);
     if (fd < 0)
  return NULL;
 
diff --git a/libc/stdio/tmpnam.c b/libc/stdio/tmpnam.c
index 1f180e0..323105b 100644
--- a/libc/stdio/tmpnam.c
+++ b/libc/stdio/tmpnam.c
@@ -41,7 +41,7 @@ tmpnam (char *s)
  0))
     return NULL;
 
-  if (__builtin_expect (__gen_tempname (tmpbuf, __GT_NOCREATE), 0))
+  if (__builtin_expect (__gen_tempname (tmpbuf, __GT_NOCREATE, 0), 0))
     return NULL;
 
   if (s == NULL)
diff --git a/libc/stdio/tmpnam_r.c b/libc/stdio/tmpnam_r.c
index eec589e..8f616b2 100644
--- a/libc/stdio/tmpnam_r.c
+++ b/libc/stdio/tmpnam_r.c
@@ -28,7 +28,7 @@ char * tmpnam_r (char *s)
 
     if (__path_search (s, L_tmpnam, NULL, NULL, 0))
  return NULL;
-    if (__gen_tempname (s, __GT_NOCREATE))
+    if (__gen_tempname (s, __GT_NOCREATE, 0))
  return NULL;
 
     return s;
diff --git a/libc/stdlib/mkdtemp.c b/libc/stdlib/mkdtemp.c
index fa9ae3b..2bf15ae 100644
--- a/libc/stdlib/mkdtemp.c
+++ b/libc/stdlib/mkdtemp.c
@@ -19,6 +19,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <sys/stat.h>
 #include "../misc/internals/tempname.h"
 
 #ifdef __USE_BSD
@@ -29,7 +30,7 @@
    (This function comes from OpenBSD.) */
 char * mkdtemp (char *template)
 {
-  if (__gen_tempname (template, __GT_DIR))
+  if (__gen_tempname (template, __GT_DIR, S_IRUSR | S_IWUSR | S_IXUSR))
     return NULL;
   else
     return template;
diff --git a/libc/stdlib/mkstemp.c b/libc/stdlib/mkstemp.c
index c569cea..ce7d7db 100644
--- a/libc/stdlib/mkstemp.c
+++ b/libc/stdlib/mkstemp.c
@@ -18,6 +18,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <sys/stat.h>
 #include "../misc/internals/tempname.h"
 
 /* Generate a unique temporary file name from TEMPLATE.
@@ -26,5 +27,5 @@
    Then open the file and return a fd. */
 int mkstemp (char *template)
 {
-    return __gen_tempname (template, __GT_FILE);
+    return __gen_tempname (template, __GT_FILE, S_IRUSR | S_IWUSR);
 }
diff --git a/libc/stdlib/mkstemp64.c b/libc/stdlib/mkstemp64.c
index 02a03f0..2cdee70 100644
--- a/libc/stdlib/mkstemp64.c
+++ b/libc/stdlib/mkstemp64.c
@@ -18,6 +18,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <sys/stat.h>
 #include "../misc/internals/tempname.h"
 
 /* Generate a unique temporary file name from TEMPLATE.
@@ -26,5 +27,5 @@
    Then open the file and return a fd. */
 int mkstemp64 (char *template)
 {
-    return __gen_tempname (template, __GT_BIGFILE);
+    return __gen_tempname (template, __GT_BIGFILE, S_IRUSR | S_IWUSR);
 }
diff --git a/libc/stdlib/mktemp.c b/libc/stdlib/mktemp.c
index f3af1c1..3c922e3 100644
--- a/libc/stdlib/mktemp.c
+++ b/libc/stdlib/mktemp.c
@@ -25,7 +25,7 @@
  * they are replaced with a string that makes the filename unique.  */
 char *mktemp(char *template)
 {
- if (__gen_tempname (template, __GT_NOCREATE) < 0)
+ if (__gen_tempname (template, __GT_NOCREATE, 0) < 0)
  /* We return the null string if we can't find a unique file name.  */
  template[0] = '\0';
 
--
1.6.5

_______________________________________________
uClibc mailing list
uClibc@...
http://lists.busybox.net/mailman/listinfo/uclibc

[Mikhail Gusarov-3]

sem_open uses mktemp to create temporary file. Reimplement it
using __gen_tmpname, removing ugly while(1) loop. As a side-effect
remove the potential source of EAGAIN errors.

Signed-off-by: Mikhail Gusarov <dottedmag@...>
---
 libpthread/nptl/sem_open.c |   37 +++++--------------------------------
 1 files changed, 5 insertions(+), 32 deletions(-)

diff --git a/libpthread/nptl/sem_open.c b/libpthread/nptl/sem_open.c
index 3bada7d..b7279fa 100644
--- a/libpthread/nptl/sem_open.c
+++ b/libpthread/nptl/sem_open.c
@@ -34,6 +34,7 @@
 #include <sys/statfs.h>
 #include <linux_fsinfo.h>
 #include "semaphoreP.h"
+#include "../../misc/internals/tempname.h"
 
 
 /* Compatibility defines. */
@@ -327,39 +328,11 @@ sem_open (const char *name, int oflag, ...)
 
       tmpfname = (char *) alloca (mountpoint.dirlen + 6 + 1);
       char *xxxxxx = mempcpy (tmpfname, mountpoint.dir, mountpoint.dirlen);
+      strcpy (xxxxxx, "XXXXXX");
 
-      int retries = 0;
-#define NRETRIES 50
-      while (1)
- {
-  /* Add the suffix for mktemp.  */
-  strcpy (xxxxxx, "XXXXXX");
-
-  /* We really want to use mktemp here.  We cannot use mkstemp
-     since the file must be opened with a specific mode.  The
-     mode cannot later be set since then we cannot apply the
-     file create mask.  */
-  if (mktemp (tmpfname) == NULL)
-    return SEM_FAILED;
-
-  /* Open the file.  Make sure we do not overwrite anything.  */
-  fd = __libc_open (tmpfname, O_RDWR | O_CREAT | O_EXCL, mode);
-  if (fd == -1)
-    {
-      if (errno == EEXIST)
- {
-  if (++retries < NRETRIES)
-    continue;
-
-  __set_errno (EAGAIN);
- }
-
-      return SEM_FAILED;
-    }
-
-  /* We got a file.  */
-  break;
- }
+      fd = __gen_tempname (tmpfname, __GT_FILE, mode);
+      if (fd == -1)
+          return SEM_FAILED;
 
       if (TEMP_FAILURE_RETRY (__libc_write (fd, &initsem, sizeof (sem_t)))
   == sizeof (sem_t)
--
1.6.5

_______________________________________________
uClibc mailing list
uClibc@...
http://lists.busybox.net/mailman/listinfo/uclibc

[Mikhail Gusarov-3]

Twas brillig at 02:33:15 08.11.2009 UTC+06 when dottedmag@... did gyre and gimble:

Whoops. I forgot to add it was for nptl_merge branch.

--
  http://fossarchy.blogspot.com/


_______________________________________________
uClibc mailing list
uClibc@...
http://lists.busybox.net/mailman/listinfo/uclibc

[Mike Frysinger]

On Saturday 07 November 2009 15:33:15 Mikhail Gusarov wrote:
>   if (__path_search(buf, L_tmpnam, NULL, NULL, 0) != 0
> - || __gen_tempname(buf, __GT_NOCREATE) != 0
> + || __gen_tempname(buf, __GT_NOCREATE, 0) != 0
>   ) {
> ...
> -extern int __gen_tempname (char *__tmpl, int __kind) attribute_hidden;
> +extern int __gen_tempname (char *__tmpl, int __kind, mode_t mode)

can you do a file size comparison between this and a varags approach ?  if
kind == __GT_NOCREATE, the mode will always be 0.  if it isnt, you can use
varargs to rip it off the stack.
-mike


_______________________________________________
uClibc mailing list
uClibc@...
http://lists.busybox.net/mailman/listinfo/uclibc

[Austin Foxley-3]

On 11/07/2009 12:33 PM, Mikhail Gusarov wrote:
> sem_open(3) needs to create a temporary file in a way which can't
> be efficiently implemented in terms of POSIX API. Extend
> __gen_tempname with mode_t mode argument in order to ease
> sem_open implementation.

Thanks for doing this. Fixed up a compile issue in the sem_open part of the
patch and applied. If you want to tweak for size as Mike mentioned, we can still
do that.

-Austin
_______________________________________________
uClibc mailing list
uClibc@...
http://lists.busybox.net/mailman/listinfo/uclibc

[Mikhail Gusarov-3]

Twas brillig at 15:48:17 09.11.2009 UTC-08 when austinf@... did gyre and gimble:

 >> sem_open(3) needs to create a temporary file in a way which can't be
 >> efficiently implemented in terms of POSIX API. Extend __gen_tempname
 >> with mode_t mode argument in order to ease sem_open implementation.

 AF> Thanks for doing this. Fixed up a compile issue in the sem_open
 AF> part of the patch and applied.

Thanks.

 AF> If you want to tweak for size as
 AF> Mike mentioned, we can still do that.

Sorry, haven't got to test it yet - swapped out by other tasks. Will
test and report as soon as possible.

--
  http://fossarchy.blogspot.com/


_______________________________________________
uClibc mailing list
uClibc@...
http://lists.busybox.net/mailman/listinfo/uclibc

[Mikhail Gusarov-3]

Twas brillig at 17:25:22 07.11.2009 UTC-05 when vapier@... did gyre and gimble:

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

 MF> can you do a file size comparison between this and a varags
 MF> approach ?  if kind == __GT_NOCREATE, the mode will always be 0.
 MF> if it isnt, you can use varargs to rip it off the stack.

Found a time to test it: varargs variant is a 32bytes longer (at least
libc.a - still yet to build a working shared library for arm eabi from
this branch).

--
  http://fossarchy.blogspot.com/


_______________________________________________
uClibc mailing list
uClibc@...
http://lists.busybox.net/mailman/listinfo/uclibc

[Mike Frysinger]

On Tuesday 17 November 2009 04:09:28 Mikhail Gusarov wrote:

> Twas brillig at 17:25:22 07.11.2009 UTC-05 gyre and gimble:
>>> -extern int __gen_tempname (char *__tmpl, int __kind) attribute_hidden;
>>> +extern int __gen_tempname (char *__tmpl, int __kind, mode_t mode)
>>
>> can you do a file size comparison between this and a varags
>> approach ?  if kind == __GT_NOCREATE, the mode will always be 0.
>> if it isnt, you can use varargs to rip it off the stack.
>
> Found a time to test it: varargs variant is a 32bytes longer (at least
> libc.a - still yet to build a working shared library for arm eabi from
> this branch).

that's fine.  thanks for checking this out.
-mike


_______________________________________________
uClibc mailing list
uClibc@...
http://lists.busybox.net/mailman/listinfo/uclibc