| Message ID | 1504211656-9263-1-git-send-email-adhemerval.zanella@linaro.org |
|---|---|
| State | Accepted |
| Commit | e0d2eb5a798613f9c24ad0056da86c8cfd19043d |
| Headers | show |
| Series | linux: Implement tmpfile with O_TMPFILE (BZ#21530) | expand |
On 08/31/2017 03:34 PM, Adhemerval Zanella wrote: > This patch adds support to use O_TMPFILE on tmpfile on Linux. This is > similar previous suggestion by Andreas Schwab [1] with the difference > the file descriptor creation is parameterized to compartmentalize Linux > only open flags (O_TMPFILE) on sysdep folder. > > Checked on x86_64-linux-gnu. > > Adhemerval Zanella <adhemerval.zanella@linaro.org> > Andreas Schwab <schwab@suse.de> > > [BZ #21530] > * include/stdio.h (__gen_tempfd): New function. > * stdio-common/Makefile (routines): Add gentempfd. > * stdio-common/gentempfd.c: New file. > * sysdeps/unix/sysv/linux/gentempfd.c: Likewise. > * stdio-common/tmpfile.c (tmpfile): First try to use a system specific > unnamed file first. Looks good to me. > [1] https://sourceware.org/ml/libc-alpha/2017-06/msg01293.html > --- > ChangeLog | 11 +++++++++++ > include/stdio.h | 3 +++ > stdio-common/Makefile | 2 +- > stdio-common/gentempfd.c | 26 ++++++++++++++++++++++++++ > stdio-common/tmpfile.c | 29 ++++++++++++++++++++--------- > sysdeps/unix/sysv/linux/gentempfd.c | 34 ++++++++++++++++++++++++++++++++++ > 6 files changed, 95 insertions(+), 10 deletions(-) > create mode 100644 stdio-common/gentempfd.c > create mode 100644 sysdeps/unix/sysv/linux/gentempfd.c > > diff --git a/include/stdio.h b/include/stdio.h > index 509447c..87e0e10 100644 > --- a/include/stdio.h > +++ b/include/stdio.h > @@ -191,5 +191,8 @@ libc_hidden_proto (__obstack_vprintf_chk) > extern FILE * __fmemopen (void *buf, size_t len, const char *mode); > libc_hidden_proto (__fmemopen) > > +extern int __gen_tempfd (int flags); > +libc_hidden_proto (__gen_tempfd) OK. > + > # endif /* not _ISOMAC */ > #endif /* stdio.h */ > diff --git a/stdio-common/Makefile b/stdio-common/Makefile > index 397e0c2..2c3c2e5 100644 > --- a/stdio-common/Makefile > +++ b/stdio-common/Makefile > @@ -39,7 +39,7 @@ routines := \ > flockfile ftrylockfile funlockfile \ > isoc99_scanf isoc99_vscanf isoc99_fscanf isoc99_vfscanf isoc99_sscanf \ > isoc99_vsscanf \ > - psiginfo > + psiginfo gentempfd OK. > > aux := errlist siglist printf-parsemb printf-parsewc fxprintf > > diff --git a/stdio-common/gentempfd.c b/stdio-common/gentempfd.c > new file mode 100644 > index 0000000..d40c57d > --- /dev/null > +++ b/stdio-common/gentempfd.c > @@ -0,0 +1,26 @@ > +/* Generate a temporary file descriptor. Generic/POSIX version. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <stdio.h> > + > +int > +__gen_tempfd (int flags) > +{ > + return -1; > +} OK. > +libc_hidden_def (__gen_tempfd) > diff --git a/stdio-common/tmpfile.c b/stdio-common/tmpfile.c > index e6030be..3e35345 100644 > --- a/stdio-common/tmpfile.c > +++ b/stdio-common/tmpfile.c > @@ -34,23 +34,34 @@ > FILE * > tmpfile (void) > { > - char buf[FILENAME_MAX]; > int fd; > FILE *f; > - > - if (__path_search (buf, FILENAME_MAX, NULL, "tmpf", 0)) > - return NULL; > int flags = 0; > #ifdef FLAGS > flags = FLAGS; > #endif > - fd = __gen_tempname (buf, 0, flags, __GT_FILE); > + > + /* First try a system specific method. */ > + fd = __gen_tempfd (flags); > + OK. > if (fd < 0) > - return NULL; > + { > + char buf[FILENAME_MAX]; > > - /* Note that this relies on the Unix semantics that > - a file is not really removed until it is closed. */ > - (void) __unlink (buf); > + if (__path_search (buf, sizeof buf, NULL, "tmpf", 0)) > + return NULL; > + > + fd = __gen_tempname (buf, 0, flags, __GT_FILE); > + if (fd < 0) > + return NULL; > + > + /* Note that this relies on the Unix semantics that > + a file is not really removed until it is closed. */ > + (void) __unlink (buf); OK. > + } > + > + if (fd < 0) > + return NULL; > > if ((f = __fdopen (fd, "w+b")) == NULL) > __close (fd); > diff --git a/sysdeps/unix/sysv/linux/gentempfd.c b/sysdeps/unix/sysv/linux/gentempfd.c > new file mode 100644 > index 0000000..902cbe2 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/gentempfd.c > @@ -0,0 +1,34 @@ > +/* Generate a temporary file descriptor. Linux version. > + Copyright (C) 2017 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <http://www.gnu.org/licenses/>. */ > + > +#include <stdio.h> > +#include <fcntl.h> > +#include <errno.h> > + > +int > +__gen_tempfd (int flags) > +{ > + int fd = __open (P_tmpdir, O_RDWR | O_TMPFILE | O_EXCL | flags, OK. I assume we don't care that on < 3.11 this will consistently fail, or < 3.15 (XFS) or < 3.16 (BTRFS). We might have remembered that failure and avoided the open(), but then again one open() is not that costly in a long sequence of syscalls that are going to create a temporary file. > + S_IRUSR | S_IWUSR); > + if (fd < 0 && errno == ENOENT && strcmp (P_tmpdir, "/tmp") != 0) > + fd = __open ("/tmp", O_RDWR | O_TMPFILE | O_EXCL | flags, > + S_IRUSR | S_IWUSR); > + > + return fd; > +} > +libc_hidden_def (__gen_tempfd) > -- Cheers, Carlos.
On Thu, Aug 31, 2017 at 05:34:16PM -0300, Adhemerval Zanella wrote: > This patch adds support to use O_TMPFILE on tmpfile on Linux. This is ... adds O_TMPFILE support to tmpfile on Linux. > similar previous suggestion by Andreas Schwab [1] with the difference similar to the previous ... with the difference that > the file descriptor creation is parameterized to compartmentalize Linux > only open flags (O_TMPFILE) on sysdep folder. ... into sysdeps. [...] > diff --git a/stdio-common/tmpfile.c b/stdio-common/tmpfile.c > index e6030be..3e35345 100644 > --- a/stdio-common/tmpfile.c > +++ b/stdio-common/tmpfile.c > @@ -34,23 +34,34 @@ > FILE * > tmpfile (void) > { > - char buf[FILENAME_MAX]; > int fd; > FILE *f; > - > - if (__path_search (buf, FILENAME_MAX, NULL, "tmpf", 0)) > - return NULL; > int flags = 0; > #ifdef FLAGS > flags = FLAGS; > #endif > - fd = __gen_tempname (buf, 0, flags, __GT_FILE); > + > + /* First try a system specific method. */ > + fd = __gen_tempfd (flags); > + > if (fd < 0) > - return NULL; > + { > + char buf[FILENAME_MAX]; > > - /* Note that this relies on the Unix semantics that > - a file is not really removed until it is closed. */ > - (void) __unlink (buf); > + if (__path_search (buf, sizeof buf, NULL, "tmpf", 0)) > + return NULL; > + > + fd = __gen_tempname (buf, 0, flags, __GT_FILE); > + if (fd < 0) > + return NULL; > + > + /* Note that this relies on the Unix semantics that > + a file is not really removed until it is closed. */ > + (void) __unlink (buf); > + } > + > + if (fd < 0) > + return NULL; The last "if (fd < 0)" check is redundant. -- ldv
On 31/08/2017 21:39, Dmitry V. Levin wrote: > On Thu, Aug 31, 2017 at 05:34:16PM -0300, Adhemerval Zanella wrote: >> This patch adds support to use O_TMPFILE on tmpfile on Linux. This is > > ... adds O_TMPFILE support to tmpfile on Linux. Ack. > >> similar previous suggestion by Andreas Schwab [1] with the difference > > similar to the previous ... with the difference that Ack. > >> the file descriptor creation is parameterized to compartmentalize Linux >> only open flags (O_TMPFILE) on sysdep folder. > > ... into sysdeps. Ack. > > [...] >> diff --git a/stdio-common/tmpfile.c b/stdio-common/tmpfile.c >> index e6030be..3e35345 100644 >> --- a/stdio-common/tmpfile.c >> +++ b/stdio-common/tmpfile.c >> @@ -34,23 +34,34 @@ >> FILE * >> tmpfile (void) >> { >> - char buf[FILENAME_MAX]; >> int fd; >> FILE *f; >> - >> - if (__path_search (buf, FILENAME_MAX, NULL, "tmpf", 0)) >> - return NULL; >> int flags = 0; >> #ifdef FLAGS >> flags = FLAGS; >> #endif >> - fd = __gen_tempname (buf, 0, flags, __GT_FILE); >> + >> + /* First try a system specific method. */ >> + fd = __gen_tempfd (flags); >> + >> if (fd < 0) >> - return NULL; >> + { >> + char buf[FILENAME_MAX]; >> >> - /* Note that this relies on the Unix semantics that >> - a file is not really removed until it is closed. */ >> - (void) __unlink (buf); >> + if (__path_search (buf, sizeof buf, NULL, "tmpf", 0)) >> + return NULL; >> + >> + fd = __gen_tempname (buf, 0, flags, __GT_FILE); >> + if (fd < 0) >> + return NULL; >> + >> + /* Note that this relies on the Unix semantics that >> + a file is not really removed until it is closed. */ >> + (void) __unlink (buf); >> + } >> + >> + if (fd < 0) >> + return NULL; > > The last "if (fd < 0)" check is redundant. Ack. I fixed the commit wording and removed the redundant tests, thanks.
Fix committed for build breakage this caused on many architectures
("implicit declaration of function 'strcmp'").
https://sourceware.org/ml/libc-testresults/2017-q3/msg00372.html
diff --git a/ChangeLog b/ChangeLog
index fccac95..5c5dd35 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,7 @@
+2017-09-01 Joseph Myers <joseph@codesourcery.com>
+
+ * sysdeps/unix/sysv/linux/gentempfd.c: Include <string.h>.
+
2017-09-01 H.J. Lu <hongjiu.lu@intel.com>
* csu/version.c (banner): Remove "by Roland McGrath et al.".
diff --git a/sysdeps/unix/sysv/linux/gentempfd.c b/sysdeps/unix/sysv/linux/gentempfd.c
index 902cbe2..ff3a649 100644
--- a/sysdeps/unix/sysv/linux/gentempfd.c
+++ b/sysdeps/unix/sysv/linux/gentempfd.c
@@ -19,6 +19,7 @@
#include <stdio.h>
#include <fcntl.h>
#include <errno.h>
+#include <string.h>
int
__gen_tempfd (int flags)
--
Joseph S. Myers
joseph@codesourcery.com
Thanks for catching it. On 01/09/2017 14:16, Joseph Myers wrote: > Fix committed for build breakage this caused on many architectures > ("implicit declaration of function 'strcmp'"). > > https://sourceware.org/ml/libc-testresults/2017-q3/msg00372.html > > diff --git a/ChangeLog b/ChangeLog > index fccac95..5c5dd35 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,7 @@ > +2017-09-01 Joseph Myers <joseph@codesourcery.com> > + > + * sysdeps/unix/sysv/linux/gentempfd.c: Include <string.h>. > + > 2017-09-01 H.J. Lu <hongjiu.lu@intel.com> > > * csu/version.c (banner): Remove "by Roland McGrath et al.". > diff --git a/sysdeps/unix/sysv/linux/gentempfd.c b/sysdeps/unix/sysv/linux/gentempfd.c > index 902cbe2..ff3a649 100644 > --- a/sysdeps/unix/sysv/linux/gentempfd.c > +++ b/sysdeps/unix/sysv/linux/gentempfd.c > @@ -19,6 +19,7 @@ > #include <stdio.h> > #include <fcntl.h> > #include <errno.h> > +#include <string.h> > > int > __gen_tempfd (int flags) >
diff --git a/include/stdio.h b/include/stdio.h index 509447c..87e0e10 100644 --- a/include/stdio.h +++ b/include/stdio.h @@ -191,5 +191,8 @@ libc_hidden_proto (__obstack_vprintf_chk) extern FILE * __fmemopen (void *buf, size_t len, const char *mode); libc_hidden_proto (__fmemopen) +extern int __gen_tempfd (int flags); +libc_hidden_proto (__gen_tempfd) + # endif /* not _ISOMAC */ #endif /* stdio.h */ diff --git a/stdio-common/Makefile b/stdio-common/Makefile index 397e0c2..2c3c2e5 100644 --- a/stdio-common/Makefile +++ b/stdio-common/Makefile @@ -39,7 +39,7 @@ routines := \ flockfile ftrylockfile funlockfile \ isoc99_scanf isoc99_vscanf isoc99_fscanf isoc99_vfscanf isoc99_sscanf \ isoc99_vsscanf \ - psiginfo + psiginfo gentempfd aux := errlist siglist printf-parsemb printf-parsewc fxprintf diff --git a/stdio-common/gentempfd.c b/stdio-common/gentempfd.c new file mode 100644 index 0000000..d40c57d --- /dev/null +++ b/stdio-common/gentempfd.c @@ -0,0 +1,26 @@ +/* Generate a temporary file descriptor. Generic/POSIX version. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <stdio.h> + +int +__gen_tempfd (int flags) +{ + return -1; +} +libc_hidden_def (__gen_tempfd) diff --git a/stdio-common/tmpfile.c b/stdio-common/tmpfile.c index e6030be..3e35345 100644 --- a/stdio-common/tmpfile.c +++ b/stdio-common/tmpfile.c @@ -34,23 +34,34 @@ FILE * tmpfile (void) { - char buf[FILENAME_MAX]; int fd; FILE *f; - - if (__path_search (buf, FILENAME_MAX, NULL, "tmpf", 0)) - return NULL; int flags = 0; #ifdef FLAGS flags = FLAGS; #endif - fd = __gen_tempname (buf, 0, flags, __GT_FILE); + + /* First try a system specific method. */ + fd = __gen_tempfd (flags); + if (fd < 0) - return NULL; + { + char buf[FILENAME_MAX]; - /* Note that this relies on the Unix semantics that - a file is not really removed until it is closed. */ - (void) __unlink (buf); + if (__path_search (buf, sizeof buf, NULL, "tmpf", 0)) + return NULL; + + fd = __gen_tempname (buf, 0, flags, __GT_FILE); + if (fd < 0) + return NULL; + + /* Note that this relies on the Unix semantics that + a file is not really removed until it is closed. */ + (void) __unlink (buf); + } + + if (fd < 0) + return NULL; if ((f = __fdopen (fd, "w+b")) == NULL) __close (fd); diff --git a/sysdeps/unix/sysv/linux/gentempfd.c b/sysdeps/unix/sysv/linux/gentempfd.c new file mode 100644 index 0000000..902cbe2 --- /dev/null +++ b/sysdeps/unix/sysv/linux/gentempfd.c @@ -0,0 +1,34 @@ +/* Generate a temporary file descriptor. Linux version. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <fcntl.h> +#include <errno.h> + +int +__gen_tempfd (int flags) +{ + int fd = __open (P_tmpdir, O_RDWR | O_TMPFILE | O_EXCL | flags, + S_IRUSR | S_IWUSR); + if (fd < 0 && errno == ENOENT && strcmp (P_tmpdir, "/tmp") != 0) + fd = __open ("/tmp", O_RDWR | O_TMPFILE | O_EXCL | flags, + S_IRUSR | S_IWUSR); + + return fd; +} +libc_hidden_def (__gen_tempfd)