From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/2685 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] Add support for mkostemp, mkstemps and mkostemps Date: Mon, 28 Jan 2013 11:33:00 -0500 Message-ID: <20130128163300.GI20323@brightrain.aerifal.cx> References: <1359349583-3643-1-git-send-email-basile@opensource.dyc.edu> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1359390800 14161 80.91.229.3 (28 Jan 2013 16:33:20 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Mon, 28 Jan 2013 16:33:20 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-2686-gllmg-musl=m.gmane.org@lists.openwall.com Mon Jan 28 17:33:39 2013 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1TzreH-0004x5-G5 for gllmg-musl@plane.gmane.org; Mon, 28 Jan 2013 17:33:33 +0100 Original-Received: (qmail 32237 invoked by uid 550); 28 Jan 2013 16:33:15 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 32229 invoked from network); 28 Jan 2013 16:33:14 -0000 Content-Disposition: inline In-Reply-To: <1359349583-3643-1-git-send-email-basile@opensource.dyc.edu> User-Agent: Mutt/1.5.21 (2010-09-15) Xref: news.gmane.org gmane.linux.lib.musl.general:2685 Archived-At: On Mon, Jan 28, 2013 at 12:06:23AM -0500, Anthony G. Basile wrote: > From: "Anthony G. Basile" > > Signed-off-by: Anthony G. Basile > --- > include/stdlib.h | 6 ++++++ > src/temp/mkostemp.c | 18 ++++++++++++++++++ > src/temp/mkostemps.c | 18 ++++++++++++++++++ > src/temp/mkstemp.c | 15 ++------------- > src/temp/mkstemps.c | 18 ++++++++++++++++++ > src/temp/mktemp.c | 7 +++---- > src/temp/randname.c | 22 ++++++++++++++++++++++ > src/temp/tempfile.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > 8 files changed, 129 insertions(+), 17 deletions(-) > create mode 100644 src/temp/mkostemp.c > create mode 100644 src/temp/mkostemps.c > create mode 100644 src/temp/mkstemps.c > create mode 100644 src/temp/randname.c > create mode 100644 src/temp/tempfile.c > > diff --git a/include/stdlib.h b/include/stdlib.h > index 671d188..4210f40 100644 > --- a/include/stdlib.h > +++ b/include/stdlib.h > @@ -95,6 +95,9 @@ 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); > +int mkstemps (char *, int); > +int mkostemps (char *, int, int); > char *mkdtemp (char *); > int getsubopt (char **, char *const *, char **); > int rand_r (unsigned *); > @@ -150,6 +153,9 @@ char *gcvt(double, int, char *); > > #if defined(_LARGEFILE64_SOURCE) || defined(_GNU_SOURCE) > #define mkstemp64 mkstemp > +#define mkostemp64 mkostemp > +#define mkstemps64 mkstemps > +#define mkostemps64 mkostemps > #endif > > #ifdef __cplusplus > diff --git a/src/temp/mkostemp.c b/src/temp/mkostemp.c > new file mode 100644 > index 0000000..750d880 > --- /dev/null > +++ b/src/temp/mkostemp.c > @@ -0,0 +1,18 @@ > +#define _GNU_SOURCE > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "libc.h" > + > +int __open_tempfile (char *, int, int); > + > +int __mkostemp(char *template, int flags) > +{ > + return __open_tempfile (template, 0, flags); > +} > + > +weak_alias(__mkostemp, mkostemp); > diff --git a/src/temp/mkostemps.c b/src/temp/mkostemps.c > new file mode 100644 > index 0000000..8c810ce > --- /dev/null > +++ b/src/temp/mkostemps.c > @@ -0,0 +1,18 @@ > +#define _GNU_SOURCE > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "libc.h" > + > +int __open_tempfile (char *, int, int); > + > +int __mkostemps(char *template, int len, int flags) > +{ > + return __open_tempfile (template, len, flags); > +} > + > +weak_alias(__mkostemps, mkostemps); Sorry, I think I wasn't clear about my comments on the alias. I was just saying the actual function can be named __mkostemps (rather than __open_tempfile or whatever) with weak_alias(__mkostemps, mkostemps); so that it also provides the publicly visible mkostemps. That avoids having an extra wrapper layer. > diff --git a/src/temp/mkstemps.c b/src/temp/mkstemps.c > new file mode 100644 > index 0000000..53fea07 > --- /dev/null > +++ b/src/temp/mkstemps.c > @@ -0,0 +1,18 @@ > +#define _GNU_SOURCE > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "libc.h" I don't think all these headers are needed for just a one-line wrapper function. :) > + > +int __open_tempfile (char *, int, int); > + > +int __mkstemps(char *template, int len) > +{ > + return __open_tempfile (template, len, O_RDWR); > +} > + > +weak_alias(__mkstemps, mkstemps); These aliases aren't needed since mkstemps, et.c aren't used internally. The __-prefixed version of a function, with the "real" name being a weak alias for it, is needed only when you want to be able to call a nonstandard function from within the standard library, or if it's being defined in the same source file as a standard function. > diff --git a/src/temp/mktemp.c b/src/temp/mktemp.c > index c0e06f5..de1afb4 100644 > --- a/src/temp/mktemp.c > +++ b/src/temp/mktemp.c > @@ -8,6 +8,8 @@ > #include > #include "libc.h" > > +char *__randname(char *); > + > char *__mktemp(char *template) > { > struct timespec ts; > @@ -21,10 +23,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); > if (access(template, F_OK) < 0) return template; > } > *template = 0; > diff --git a/src/temp/randname.c b/src/temp/randname.c > new file mode 100644 > index 0000000..4d3476f > --- /dev/null > +++ b/src/temp/randname.c > @@ -0,0 +1,22 @@ > +#include > +#include > +#include > +#include > +#include > +#include "libc.h" > + > +char *__randname(char *template) > +{ > + struct timespec ts; > + size_t i, l = strlen(template); > + unsigned long r; > + > + /* This assumes that a check for the template > + size has alrady been made */ > + 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); > + > + return template; > +} > diff --git a/src/temp/tempfile.c b/src/temp/tempfile.c > new file mode 100644 > index 0000000..93808a6 > --- /dev/null > +++ b/src/temp/tempfile.c > @@ -0,0 +1,42 @@ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "libc.h" > + > +char *__randname(char *); > + > +int __open_tempfile (char *template, int len, int flags) > +{ > + if (len < 0) return EINVAL; > + > + int l = strlen(template)-len; > + if (l < 6 || strncmp(template+l-6, "XXXXXX",6)) { > + errno = EINVAL; > + *template = 0; > + return -1; > + } > + > + /* Null terminate the template before the suffix, > + and save the char for adding back the suffix */ > + char suffix = template[l]; > + template[l] = '\0'; > + > + int fd, retries = 100, t0 = *template; > + while (retries--) { > + if (!*__randname(template)) return -1; > + /* Add back the suffix */ > + template[l] = suffix; > + if ((fd = open(template, flags | 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+l-6, "XXXXXX"); Since your __randname is not checking for XXXXXX like __mktemp was doing, this strcpy is probably not needed. I'm not sure whether it would be cleaner to keep this strcpy and put the template check back in __randname (so that mktemp and __mkostemps don't duplicate the check) or just get rid of the strcpy here. What do you think? Rich