From: Rich Felker <dalias@aerifal.cx>
To: musl@lists.openwall.com
Subject: Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps
Date: Sat, 2 Feb 2013 22:30:42 -0500 [thread overview]
Message-ID: <20130203033042.GZ20323@brightrain.aerifal.cx> (raw)
In-Reply-To: <1359839705-25965-1-git-send-email-basile@opensource.dyc.edu>
[-- Attachment #1: Type: text/plain, Size: 1791 bytes --]
I'm attaching my "proposed" version, which isn't quite in the state I
want it to be committed, but it addresses a few issues, some of which
I tried to explain before but which are difficult without just showing
it...
Some notes:
- The public header in which a public function is defined should
always be included in the file with the definition, to ensure the
prototype matches if nothing else.
- Nonstandard functions should be protected in the headers, not
exposed in the default namespace. Technically mkostemp is
nonstandard, but it's accepted for inclusion in Issue 8, and so far
the policy for such functions is to treat them as if they're already
in the standard.
- Using the LFS64 macro when _GNU_SOURCE is defined breaks due to the
#defines in the system headers. This is a bug in musl, but I worked
around it by using _BSD_SOURCE instead which doesn't expose the *64
junk.
- O_RDWR needs to be builtin to the tempfile-creation core (now
__mkostemps), not passed in by the caller. The flags argument is
only for additional flags, and the proposed text in POSIX is clear
that passing anything but a small limited set of flags yields
unspecified behavior.
- I'm using __clock_gettime rather than clock_gettime so that these
functions can hopefully eventually be shared with the code for
tmpfile and tmpnam, which are in ISO C and can't depend on POSIX
namespace. However the issue of using open still remains; it should
perhaps be changed to make the syscall directly like tmpfile does
now. This can be changed later; it's not a big deal.
I have not addressed any of the randomness considerations; I just
copied the code for that from your latest patch. Comments on the ml
should still be taken into consideration for finalizing that.
Rich
[-- Attachment #2: mkostemps.diff --]
[-- Type: text/plain, Size: 5017 bytes --]
diff --git a/include/stdlib.h b/include/stdlib.h
index 671d188..0bcc9f4 100644
--- a/include/stdlib.h
+++ b/include/stdlib.h
@@ -95,6 +95,7 @@ int posix_memalign (void **, size_t, size_t);
int setenv (const char *, const char *, int);
int unsetenv (const char *);
int mkstemp (char *);
+int mkostemp (char *, int);
char *mkdtemp (char *);
int getsubopt (char **, char *const *, char **);
int rand_r (unsigned *);
@@ -134,6 +135,8 @@ void lcong48 (unsigned short [7]);
#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
#include <alloca.h>
char *mktemp (char *);
+int mkstemps (char *, int);
+int mkostemps (char *, int, int);
void *valloc (size_t);
void *memalign(size_t, size_t);
#define WCOREDUMP(s) ((s) & 0x80)
@@ -150,6 +153,11 @@ char *gcvt(double, int, char *);
#if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE)
#define mkstemp64 mkstemp
+#define mkostemp64 mkostemp
+#if defined(_GNU_SOURCE) || defined(_BSD_SOURCE)
+#define mkstemps64 mkstemps
+#define mkostemps64 mkostemps
+#endif
#endif
#ifdef __cplusplus
diff --git a/src/temp/__randname.c b/src/temp/__randname.c
new file mode 100644
index 0000000..b097576
--- /dev/null
+++ b/src/temp/__randname.c
@@ -0,0 +1,21 @@
+#include <string.h>
+#include <time.h>
+#include <stdint.h>
+
+int __clock_gettime(clockid_t, struct timespec *);
+
+/* This assumes that a check for the
+ template size has alrady been made */
+char *__randname(char *template)
+{
+ int i;
+ struct timespec ts;
+ unsigned long r;
+
+ __clock_gettime(CLOCK_REALTIME, &ts);
+ r = ts.tv_nsec*65537 ^ (uintptr_t)&ts / 16 + (uintptr_t)template;
+ for (i=0; i<6; i++, r>>=5)
+ template[i] = 'A'+(r&15)+(r&16)*2;
+
+ return template;
+}
diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c
new file mode 100644
index 0000000..e73e22a
--- /dev/null
+++ b/src/temp/mkostemp.c
@@ -0,0 +1,12 @@
+#define _BSD_SOURCE
+#include <stdlib.h>
+#include "libc.h"
+
+int __mkostemps(char *, int, int);
+
+int mkostemp(char *template, int flags)
+{
+ return __mkostemps(template, 0, flags);
+}
+
+LFS64(mkostemp);
diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c
new file mode 100644
index 0000000..804a547
--- /dev/null
+++ b/src/temp/mkostemps.c
@@ -0,0 +1,32 @@
+#define _BSD_SOURCE
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+#include "libc.h"
+
+char *__randname(char *);
+
+int __mkostemps(char *template, int len, int flags)
+{
+ if (len < 0) return EINVAL;
+
+ size_t l = strlen(template)-len;
+ if (l < 6 || strncmp(template+l-6, "XXXXXX", 6)) {
+ errno = EINVAL;
+ *template = 0;
+ return -1;
+ }
+
+ int fd, retries = 100;
+ while (retries--) {
+ __randname(template+l-6);
+ if ((fd = open(template, flags | O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
+ return fd;
+ if (errno != EEXIST) return -1;
+ }
+ return -1;
+}
+
+weak_alias(__mkostemps, mkostemps);
+weak_alias(__mkostemps, mkostemps64);
diff --git a/src/temp/mkstemp.c b/src/temp/mkstemp.c
index a390d42..85764af 100644
--- a/src/temp/mkstemp.c
+++ b/src/temp/mkstemp.c
@@ -1,28 +1,11 @@
-#include <string.h>
-#include <stdio.h>
#include <stdlib.h>
-#include <fcntl.h>
-#include <unistd.h>
-#include <limits.h>
-#include <errno.h>
#include "libc.h"
-char *__mktemp(char *);
+int __mkostemps(char *, int, int);
int mkstemp(char *template)
{
- int fd, retries = 100, t0 = *template;
- while (retries--) {
- if (!*__mktemp(template)) return -1;
- if ((fd = open(template, O_RDWR | O_CREAT | O_EXCL, 0600))>=0)
- return fd;
- if (errno != EEXIST) return -1;
- /* this is safe because mktemp verified
- * that we have a valid template string */
- template[0] = t0;
- strcpy(template+strlen(template)-6, "XXXXXX");
- }
- return -1;
+ return __mkostemps(template, 0, 0);
}
LFS64(mkstemp);
diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c
new file mode 100644
index 0000000..fda710b
--- /dev/null
+++ b/src/temp/mkstemps.c
@@ -0,0 +1,12 @@
+#define _BSD_SOURCE
+#include <stdlib.h>
+#include "libc.h"
+
+int __mkostemps(char *, int, int);
+
+int mkstemps(char *template, int len)
+{
+ return __mkostemps(template, len, 0);
+}
+
+LFS64(mkstemps);
diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c
index c0e06f5..ed2c103 100644
--- a/src/temp/mktemp.c
+++ b/src/temp/mktemp.c
@@ -1,17 +1,14 @@
#include <string.h>
-#include <stdio.h>
-#include <stdlib.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
-#include <time.h>
-#include <stdint.h>
#include "libc.h"
+char *__randname(char *);
+
char *__mktemp(char *template)
{
- struct timespec ts;
- size_t i, l = strlen(template);
+ size_t l = strlen(template);
int retries = 10000;
unsigned long r;
@@ -21,10 +18,7 @@ char *__mktemp(char *template)
return template;
}
while (retries--) {
- clock_gettime(CLOCK_REALTIME, &ts);
- r = ts.tv_nsec + (uintptr_t)&ts / 16 + (uintptr_t)template;
- for (i=1; i<=6; i++, r>>=4)
- template[l-i] = 'A'+(r&15);
+ __randname(template+l-6);
if (access(template, F_OK) < 0) return template;
}
*template = 0;
next prev parent reply other threads:[~2013-02-03 3:30 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-02 21:15 Anthony G. Basile
2013-02-02 21:17 ` Anthony G. Basile
2013-02-03 3:30 ` Rich Felker [this message]
-- strict thread matches above, loose matches on Subject: below --
2013-02-02 18:45 Anthony G. Basile
2013-02-02 19:51 ` John Spencer
2013-02-02 19:59 ` John Spencer
2013-02-02 22:11 ` Szabolcs Nagy
2013-02-02 20:14 ` Szabolcs Nagy
2013-02-02 20:38 ` Anthony G. Basile
2013-02-02 22:22 ` Szabolcs Nagy
2013-01-28 5:06 Anthony G. Basile
2013-01-28 8:47 ` John Spencer
2013-01-28 9:37 ` Szabolcs Nagy
2013-01-28 17:33 ` Szabolcs Nagy
2013-01-29 23:16 ` Anthony G. Basile
2013-01-30 7:21 ` Rich Felker
2013-01-30 7:59 ` Hardy Falk
2013-01-30 13:45 ` Szabolcs Nagy
2013-01-30 16:51 ` Rich Felker
2013-01-30 19:12 ` Szabolcs Nagy
2013-01-30 19:22 ` Rich Felker
2013-01-31 0:28 ` Szabolcs Nagy
2013-01-28 16:33 ` Rich Felker
2013-02-02 3:48 ` Rich Felker
2013-02-02 18:46 ` Anthony G. Basile
2013-01-28 2:36 Anthony G. Basile
2013-01-28 5:01 ` Anthony G. Basile
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130203033042.GZ20323@brightrain.aerifal.cx \
--to=dalias@aerifal.cx \
--cc=musl@lists.openwall.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/musl/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).