| Message ID | 1499977587-28794-1-git-send-email-adhemerval.zanella@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
Ping. On 13/07/2017 17:26, Adhemerval Zanella wrote: > As POSIX states [1] a freopen call should first flush the stream as > if by a call fflush. C99 (n1256) and C11 (n1570) only states the > function should first close any file associated with the specific > stream. Although current implementation only follow C specification, > current BSD and other libc implementation (musl) are in sync with > POSIX and fflush the stream. > > This patch change freopen{64} to fflush the stream before actually > reopening it (or returning if the stream does not support reopen). > It also changes the Linux implementation to avoid a dynamic > allocation on 'fd_to_filename'. > > Checked on x86_64-linux-gnu. > > From Florian's comment, this is intended for 2.27. This patch should > be applied on top on 'libio: Fix open_memstream fflush (NULL) (BZ#21735)' [2]. > > [BZ #21037] > * libio/freopen.c (freopen): Sync stream before reopen and adjust to > new fd_to_filename interface. > * libio/freopen64.c (freopen64): Likewise. > * sysdeps/generic/fd_to_filename.h (fd_to_filename): Change signature. > * sysdeps/unix/sysv/linux/fd_to_filename.h (fd_to_filename): Likewise > and remove internal dynamic allocation. > * libio/Makefile (tests): Add tst-memstream5 and tst-wmemstream5. > * libio/tst-memstream.h (FPUTS): New macro. > * libio/tst-memstream4.c: New file. > * libio/tst-wmemstream4.c: Likewise. > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/ > [2] https://sourceware.org/ml/libc-alpha/2017-07/msg00548.html > --- > ChangeLog | 14 ++++++++ > libio/Makefile | 3 +- > libio/freopen.c | 33 ++++++++++------- > libio/freopen64.c | 30 ++++++++++------ > libio/tst-memstream.h | 2 ++ > libio/tst-memstream5.c | 62 ++++++++++++++++++++++++++++++++ > libio/tst-wmemstream5.c | 20 +++++++++++ > sysdeps/generic/fd_to_filename.h | 6 ++-- > sysdeps/unix/sysv/linux/fd_to_filename.h | 31 +++++----------- > 9 files changed, 151 insertions(+), 50 deletions(-) > create mode 100644 libio/tst-memstream5.c > create mode 100644 libio/tst-wmemstream5.c > > diff --git a/libio/Makefile b/libio/Makefile > index 6ee809a..8adf2d8 100644 > --- a/libio/Makefile > +++ b/libio/Makefile > @@ -58,8 +58,9 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ > tst-mmap2-eofsync tst-mmap-offend bug-fopena+ bug-wfflush \ > bug-ungetc2 bug-ftell bug-ungetc3 bug-ungetc4 tst-fopenloc2 \ > tst-memstream1 tst-memstream2 tst-memstream3 tst-memstream4 \ > + tst-memstream5 \ > tst-wmemstream1 tst-wmemstream2 tst-wmemstream3 tst-wmemstream4 \ > - bug-memstream1 bug-wmemstream1 \ > + tst-wmemstream5 bug-memstream1 bug-wmemstream1 \ > tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \ > tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \ > tst-ftell-append tst-fputws > diff --git a/libio/freopen.c b/libio/freopen.c > index 980523a..505e77d 100644 > --- a/libio/freopen.c > +++ b/libio/freopen.c > @@ -24,28 +24,38 @@ > This exception applies to code released by its copyright holders > in files containing the exception. */ > > -#include "libioP.h" > -#include "stdio.h" > +#include <stdio.h> > #include <fcntl.h> > #include <stdlib.h> > #include <unistd.h> > > -#include <shlib-compat.h> > +#include <libioP.h> > #include <fd_to_filename.h> > - > -#include <kernel-features.h> > +#include <shlib-compat.h> > > FILE * > freopen (const char *filename, const char *mode, FILE *fp) > { > - FILE *result; > + FILE *result = NULL; > + char fdfilename[30]; > + > CHECK_FILE (fp, NULL); > - if (!(fp->_flags & _IO_IS_FILEBUF)) > - return NULL; > + > _IO_acquire_lock (fp); > + /* First flush the stream (failure should be ignored). */ > + _IO_SYNC (fp); > + > + if (!(fp->_flags & _IO_IS_FILEBUF)) > + goto end; > + > int fd = _IO_fileno (fp); > - const char *gfilename = (filename == NULL && fd >= 0 > - ? fd_to_filename (fd) : filename); > + const char *gfilename; > + if (filename == NULL && fd >= 0) > + gfilename = fd_to_filename (fd, fdfilename, sizeof fdfilename) > + ? fdfilename : NULL; > + else > + gfilename = filename; > + > fp->_flags2 |= _IO_FLAGS2_NOCLOSE; > #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) > if (&_IO_stdin_used == NULL) > @@ -101,9 +111,6 @@ freopen (const char *filename, const char *mode, FILE *fp) > __close (fd); > > end: > - if (filename == NULL) > - free ((char *) gfilename); > - > _IO_release_lock (fp); > return result; > } > diff --git a/libio/freopen64.c b/libio/freopen64.c > index 1e56de6..28203c9 100644 > --- a/libio/freopen64.c > +++ b/libio/freopen64.c > @@ -24,27 +24,37 @@ > This exception applies to code released by its copyright holders > in files containing the exception. */ > > -#include "libioP.h" > -#include "stdio.h" > +#include <stdio.h> > #include <fcntl.h> > #include <stdlib.h> > #include <unistd.h> > > +#include <libioP.h> > #include <fd_to_filename.h> > > -#include <kernel-features.h> > - > FILE * > freopen64 (const char *filename, const char *mode, FILE *fp) > { > - FILE *result; > + FILE *result = NULL; > + char fdfilename[30]; > + > CHECK_FILE (fp, NULL); > - if (!(fp->_flags & _IO_IS_FILEBUF)) > - return NULL; > + > _IO_acquire_lock (fp); > + /* First flush the stream (failure should be ignored). */ > + _IO_SYNC (fp); > + > + if (!(fp->_flags & _IO_IS_FILEBUF)) > + goto end; > + > int fd = _IO_fileno (fp); > - const char *gfilename = (filename == NULL && fd >= 0 > - ? fd_to_filename (fd) : filename); > + const char *gfilename; > + if (filename == NULL && fd >= 0) > + gfilename = fd_to_filename (fd, fdfilename, sizeof fdfilename) > + ? fdfilename : NULL; > + else > + gfilename = filename; > + > fp->_flags2 |= _IO_FLAGS2_NOCLOSE; > _IO_file_close_it (fp); > _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps; > @@ -84,8 +94,6 @@ freopen64 (const char *filename, const char *mode, FILE *fp) > __close (fd); > > end: > - if (filename == NULL) > - free ((char *) gfilename); > _IO_release_lock (fp); > return result; > } > diff --git a/libio/tst-memstream.h b/libio/tst-memstream.h > index ffc2a65..86ea0d7 100644 > --- a/libio/tst-memstream.h > +++ b/libio/tst-memstream.h > @@ -49,6 +49,7 @@ fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq) > # define PRINTF wprintf > # define FWRITE fwwrite > # define FPUTC fputwc > +# define FPUTS fputws > # define STRCMP wcscmp > # define STRLEN wcslen > #else > @@ -58,6 +59,7 @@ fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq) > # define PRINTF printf > # define FWRITE fwrite > # define FPUTC fputc > +# define FPUTS fputs > # define STRCMP strcmp > # define STRLEN strlen > #endif > diff --git a/libio/tst-memstream5.c b/libio/tst-memstream5.c > new file mode 100644 > index 0000000..7c1f37c > --- /dev/null > +++ b/libio/tst-memstream5.c > @@ -0,0 +1,62 @@ > +/* Test for open_memstream BZ #21037. > + 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 "tst-memstream.h" > + > +static void > +mcheck_abort (enum mcheck_status ev) > +{ > + printf ("mecheck failed with status %d\n", (int) ev); > + exit (1); > +} > + > +static int > +do_test (void) > +{ > + mcheck_pedantic (mcheck_abort); > + > + /* Check if freopen proper fflush the stream. */ > + { > + CHAR_T old[] = W("old"); > + CHAR_T *buf = old; > + size_t size; > + > + FILE *fp = OPEN_MEMSTREAM (&buf, &size); > + TEST_VERIFY_EXIT (fp != NULL); > + > + FPUTS (W("new"), fp); > + /* The stream buffer pointer should be updated with only a fflush or > + fclose. */ > + TEST_VERIFY (STRCMP (buf, old) == 0); > + > + /* The old stream should be fflush the stream, even for an invalid > + streams. */ > + FILE *nfp = freopen ("invalid-file", "r", fp); > + TEST_VERIFY (nfp == NULL); > + > + TEST_VERIFY (STRCMP (buf, W("new")) == 0); > + > + TEST_VERIFY (fclose (fp) == 0); > + > + free (buf); > + } > + > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/libio/tst-wmemstream5.c b/libio/tst-wmemstream5.c > new file mode 100644 > index 0000000..4e87289 > --- /dev/null > +++ b/libio/tst-wmemstream5.c > @@ -0,0 +1,20 @@ > +/* Test for open_wmemstream BZ #21037. > + 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/>. */ > + > +#define TEST_WCHAR > +#include <libio/tst-memstream5.c> > diff --git a/sysdeps/generic/fd_to_filename.h b/sysdeps/generic/fd_to_filename.h > index fe15f5d..db594f2 100644 > --- a/sysdeps/generic/fd_to_filename.h > +++ b/sysdeps/generic/fd_to_filename.h > @@ -18,8 +18,8 @@ > > /* In general there is no generic way to query filename for an open > file descriptor. */ > -static inline const char * > -fd_to_filename (int fd) > +static inline bool > +fd_to_filename (int fd, char *buf, size_t len) > { > - return NULL; > + return false; > } > diff --git a/sysdeps/unix/sysv/linux/fd_to_filename.h b/sysdeps/unix/sysv/linux/fd_to_filename.h > index 414a30f..6cd0bdd 100644 > --- a/sysdeps/unix/sysv/linux/fd_to_filename.h > +++ b/sysdeps/unix/sysv/linux/fd_to_filename.h > @@ -16,30 +16,17 @@ > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > > -#include <stdlib.h> > -#include <string.h> > +#include <stdio.h> > #include <sys/stat.h> > -#include <_itoa.h> > > -static inline const char * > -fd_to_filename (int fd) > +static inline bool > +fd_to_filename (int fd, char *buf, size_t len) > { > - char *ret = malloc (30); > + __snprintf (buf, len, "/proc/self/fd/%d", fd); > > - if (ret != NULL) > - { > - struct stat64 st; > - > - *_fitoa_word (fd, __stpcpy (ret, "/proc/self/fd/"), 10, 0) = '\0'; > - > - /* We must make sure the file exists. */ > - if (__lxstat64 (_STAT_VER, ret, &st) < 0) > - { > - /* /proc is not mounted or something else happened. Don't > - return the file name. */ > - free (ret); > - ret = NULL; > - } > - } > - return ret; > + /* We must make sure the file exists. */ > + if (__lxstat64 (_STAT_VER, buf, & (struct stat64) {}) < 0) > + /* /proc is not mounted or something else happened. */ > + return false; > + return true; > } >
Ping (x2). On 08/08/2017 10:52, Adhemerval Zanella wrote: > Ping. > > On 13/07/2017 17:26, Adhemerval Zanella wrote: >> As POSIX states [1] a freopen call should first flush the stream as >> if by a call fflush. C99 (n1256) and C11 (n1570) only states the >> function should first close any file associated with the specific >> stream. Although current implementation only follow C specification, >> current BSD and other libc implementation (musl) are in sync with >> POSIX and fflush the stream. >> >> This patch change freopen{64} to fflush the stream before actually >> reopening it (or returning if the stream does not support reopen). >> It also changes the Linux implementation to avoid a dynamic >> allocation on 'fd_to_filename'. >> >> Checked on x86_64-linux-gnu. >> >> From Florian's comment, this is intended for 2.27. This patch should >> be applied on top on 'libio: Fix open_memstream fflush (NULL) (BZ#21735)' [2]. >> >> [BZ #21037] >> * libio/freopen.c (freopen): Sync stream before reopen and adjust to >> new fd_to_filename interface. >> * libio/freopen64.c (freopen64): Likewise. >> * sysdeps/generic/fd_to_filename.h (fd_to_filename): Change signature. >> * sysdeps/unix/sysv/linux/fd_to_filename.h (fd_to_filename): Likewise >> and remove internal dynamic allocation. >> * libio/Makefile (tests): Add tst-memstream5 and tst-wmemstream5. >> * libio/tst-memstream.h (FPUTS): New macro. >> * libio/tst-memstream4.c: New file. >> * libio/tst-wmemstream4.c: Likewise. >> >> [1] http://pubs.opengroup.org/onlinepubs/9699919799/ >> [2] https://sourceware.org/ml/libc-alpha/2017-07/msg00548.html >> --- >> ChangeLog | 14 ++++++++ >> libio/Makefile | 3 +- >> libio/freopen.c | 33 ++++++++++------- >> libio/freopen64.c | 30 ++++++++++------ >> libio/tst-memstream.h | 2 ++ >> libio/tst-memstream5.c | 62 ++++++++++++++++++++++++++++++++ >> libio/tst-wmemstream5.c | 20 +++++++++++ >> sysdeps/generic/fd_to_filename.h | 6 ++-- >> sysdeps/unix/sysv/linux/fd_to_filename.h | 31 +++++----------- >> 9 files changed, 151 insertions(+), 50 deletions(-) >> create mode 100644 libio/tst-memstream5.c >> create mode 100644 libio/tst-wmemstream5.c >> >> diff --git a/libio/Makefile b/libio/Makefile >> index 6ee809a..8adf2d8 100644 >> --- a/libio/Makefile >> +++ b/libio/Makefile >> @@ -58,8 +58,9 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ >> tst-mmap2-eofsync tst-mmap-offend bug-fopena+ bug-wfflush \ >> bug-ungetc2 bug-ftell bug-ungetc3 bug-ungetc4 tst-fopenloc2 \ >> tst-memstream1 tst-memstream2 tst-memstream3 tst-memstream4 \ >> + tst-memstream5 \ >> tst-wmemstream1 tst-wmemstream2 tst-wmemstream3 tst-wmemstream4 \ >> - bug-memstream1 bug-wmemstream1 \ >> + tst-wmemstream5 bug-memstream1 bug-wmemstream1 \ >> tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \ >> tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \ >> tst-ftell-append tst-fputws >> diff --git a/libio/freopen.c b/libio/freopen.c >> index 980523a..505e77d 100644 >> --- a/libio/freopen.c >> +++ b/libio/freopen.c >> @@ -24,28 +24,38 @@ >> This exception applies to code released by its copyright holders >> in files containing the exception. */ >> >> -#include "libioP.h" >> -#include "stdio.h" >> +#include <stdio.h> >> #include <fcntl.h> >> #include <stdlib.h> >> #include <unistd.h> >> >> -#include <shlib-compat.h> >> +#include <libioP.h> >> #include <fd_to_filename.h> >> - >> -#include <kernel-features.h> >> +#include <shlib-compat.h> >> >> FILE * >> freopen (const char *filename, const char *mode, FILE *fp) >> { >> - FILE *result; >> + FILE *result = NULL; >> + char fdfilename[30]; >> + >> CHECK_FILE (fp, NULL); >> - if (!(fp->_flags & _IO_IS_FILEBUF)) >> - return NULL; >> + >> _IO_acquire_lock (fp); >> + /* First flush the stream (failure should be ignored). */ >> + _IO_SYNC (fp); >> + >> + if (!(fp->_flags & _IO_IS_FILEBUF)) >> + goto end; >> + >> int fd = _IO_fileno (fp); >> - const char *gfilename = (filename == NULL && fd >= 0 >> - ? fd_to_filename (fd) : filename); >> + const char *gfilename; >> + if (filename == NULL && fd >= 0) >> + gfilename = fd_to_filename (fd, fdfilename, sizeof fdfilename) >> + ? fdfilename : NULL; >> + else >> + gfilename = filename; >> + >> fp->_flags2 |= _IO_FLAGS2_NOCLOSE; >> #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) >> if (&_IO_stdin_used == NULL) >> @@ -101,9 +111,6 @@ freopen (const char *filename, const char *mode, FILE *fp) >> __close (fd); >> >> end: >> - if (filename == NULL) >> - free ((char *) gfilename); >> - >> _IO_release_lock (fp); >> return result; >> } >> diff --git a/libio/freopen64.c b/libio/freopen64.c >> index 1e56de6..28203c9 100644 >> --- a/libio/freopen64.c >> +++ b/libio/freopen64.c >> @@ -24,27 +24,37 @@ >> This exception applies to code released by its copyright holders >> in files containing the exception. */ >> >> -#include "libioP.h" >> -#include "stdio.h" >> +#include <stdio.h> >> #include <fcntl.h> >> #include <stdlib.h> >> #include <unistd.h> >> >> +#include <libioP.h> >> #include <fd_to_filename.h> >> >> -#include <kernel-features.h> >> - >> FILE * >> freopen64 (const char *filename, const char *mode, FILE *fp) >> { >> - FILE *result; >> + FILE *result = NULL; >> + char fdfilename[30]; >> + >> CHECK_FILE (fp, NULL); >> - if (!(fp->_flags & _IO_IS_FILEBUF)) >> - return NULL; >> + >> _IO_acquire_lock (fp); >> + /* First flush the stream (failure should be ignored). */ >> + _IO_SYNC (fp); >> + >> + if (!(fp->_flags & _IO_IS_FILEBUF)) >> + goto end; >> + >> int fd = _IO_fileno (fp); >> - const char *gfilename = (filename == NULL && fd >= 0 >> - ? fd_to_filename (fd) : filename); >> + const char *gfilename; >> + if (filename == NULL && fd >= 0) >> + gfilename = fd_to_filename (fd, fdfilename, sizeof fdfilename) >> + ? fdfilename : NULL; >> + else >> + gfilename = filename; >> + >> fp->_flags2 |= _IO_FLAGS2_NOCLOSE; >> _IO_file_close_it (fp); >> _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps; >> @@ -84,8 +94,6 @@ freopen64 (const char *filename, const char *mode, FILE *fp) >> __close (fd); >> >> end: >> - if (filename == NULL) >> - free ((char *) gfilename); >> _IO_release_lock (fp); >> return result; >> } >> diff --git a/libio/tst-memstream.h b/libio/tst-memstream.h >> index ffc2a65..86ea0d7 100644 >> --- a/libio/tst-memstream.h >> +++ b/libio/tst-memstream.h >> @@ -49,6 +49,7 @@ fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq) >> # define PRINTF wprintf >> # define FWRITE fwwrite >> # define FPUTC fputwc >> +# define FPUTS fputws >> # define STRCMP wcscmp >> # define STRLEN wcslen >> #else >> @@ -58,6 +59,7 @@ fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq) >> # define PRINTF printf >> # define FWRITE fwrite >> # define FPUTC fputc >> +# define FPUTS fputs >> # define STRCMP strcmp >> # define STRLEN strlen >> #endif >> diff --git a/libio/tst-memstream5.c b/libio/tst-memstream5.c >> new file mode 100644 >> index 0000000..7c1f37c >> --- /dev/null >> +++ b/libio/tst-memstream5.c >> @@ -0,0 +1,62 @@ >> +/* Test for open_memstream BZ #21037. >> + 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 "tst-memstream.h" >> + >> +static void >> +mcheck_abort (enum mcheck_status ev) >> +{ >> + printf ("mecheck failed with status %d\n", (int) ev); >> + exit (1); >> +} >> + >> +static int >> +do_test (void) >> +{ >> + mcheck_pedantic (mcheck_abort); >> + >> + /* Check if freopen proper fflush the stream. */ >> + { >> + CHAR_T old[] = W("old"); >> + CHAR_T *buf = old; >> + size_t size; >> + >> + FILE *fp = OPEN_MEMSTREAM (&buf, &size); >> + TEST_VERIFY_EXIT (fp != NULL); >> + >> + FPUTS (W("new"), fp); >> + /* The stream buffer pointer should be updated with only a fflush or >> + fclose. */ >> + TEST_VERIFY (STRCMP (buf, old) == 0); >> + >> + /* The old stream should be fflush the stream, even for an invalid >> + streams. */ >> + FILE *nfp = freopen ("invalid-file", "r", fp); >> + TEST_VERIFY (nfp == NULL); >> + >> + TEST_VERIFY (STRCMP (buf, W("new")) == 0); >> + >> + TEST_VERIFY (fclose (fp) == 0); >> + >> + free (buf); >> + } >> + >> + return 0; >> +} >> + >> +#include <support/test-driver.c> >> diff --git a/libio/tst-wmemstream5.c b/libio/tst-wmemstream5.c >> new file mode 100644 >> index 0000000..4e87289 >> --- /dev/null >> +++ b/libio/tst-wmemstream5.c >> @@ -0,0 +1,20 @@ >> +/* Test for open_wmemstream BZ #21037. >> + 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/>. */ >> + >> +#define TEST_WCHAR >> +#include <libio/tst-memstream5.c> >> diff --git a/sysdeps/generic/fd_to_filename.h b/sysdeps/generic/fd_to_filename.h >> index fe15f5d..db594f2 100644 >> --- a/sysdeps/generic/fd_to_filename.h >> +++ b/sysdeps/generic/fd_to_filename.h >> @@ -18,8 +18,8 @@ >> >> /* In general there is no generic way to query filename for an open >> file descriptor. */ >> -static inline const char * >> -fd_to_filename (int fd) >> +static inline bool >> +fd_to_filename (int fd, char *buf, size_t len) >> { >> - return NULL; >> + return false; >> } >> diff --git a/sysdeps/unix/sysv/linux/fd_to_filename.h b/sysdeps/unix/sysv/linux/fd_to_filename.h >> index 414a30f..6cd0bdd 100644 >> --- a/sysdeps/unix/sysv/linux/fd_to_filename.h >> +++ b/sysdeps/unix/sysv/linux/fd_to_filename.h >> @@ -16,30 +16,17 @@ >> License along with the GNU C Library; if not, see >> <http://www.gnu.org/licenses/>. */ >> >> -#include <stdlib.h> >> -#include <string.h> >> +#include <stdio.h> >> #include <sys/stat.h> >> -#include <_itoa.h> >> >> -static inline const char * >> -fd_to_filename (int fd) >> +static inline bool >> +fd_to_filename (int fd, char *buf, size_t len) >> { >> - char *ret = malloc (30); >> + __snprintf (buf, len, "/proc/self/fd/%d", fd); >> >> - if (ret != NULL) >> - { >> - struct stat64 st; >> - >> - *_fitoa_word (fd, __stpcpy (ret, "/proc/self/fd/"), 10, 0) = '\0'; >> - >> - /* We must make sure the file exists. */ >> - if (__lxstat64 (_STAT_VER, ret, &st) < 0) >> - { >> - /* /proc is not mounted or something else happened. Don't >> - return the file name. */ >> - free (ret); >> - ret = NULL; >> - } >> - } >> - return ret; >> + /* We must make sure the file exists. */ >> + if (__lxstat64 (_STAT_VER, buf, & (struct stat64) {}) < 0) >> + /* /proc is not mounted or something else happened. */ >> + return false; >> + return true; >> } >>
diff --git a/libio/Makefile b/libio/Makefile index 6ee809a..8adf2d8 100644 --- a/libio/Makefile +++ b/libio/Makefile @@ -58,8 +58,9 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc \ tst-mmap2-eofsync tst-mmap-offend bug-fopena+ bug-wfflush \ bug-ungetc2 bug-ftell bug-ungetc3 bug-ungetc4 tst-fopenloc2 \ tst-memstream1 tst-memstream2 tst-memstream3 tst-memstream4 \ + tst-memstream5 \ tst-wmemstream1 tst-wmemstream2 tst-wmemstream3 tst-wmemstream4 \ - bug-memstream1 bug-wmemstream1 \ + tst-wmemstream5 bug-memstream1 bug-wmemstream1 \ tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \ tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \ tst-ftell-append tst-fputws diff --git a/libio/freopen.c b/libio/freopen.c index 980523a..505e77d 100644 --- a/libio/freopen.c +++ b/libio/freopen.c @@ -24,28 +24,38 @@ This exception applies to code released by its copyright holders in files containing the exception. */ -#include "libioP.h" -#include "stdio.h" +#include <stdio.h> #include <fcntl.h> #include <stdlib.h> #include <unistd.h> -#include <shlib-compat.h> +#include <libioP.h> #include <fd_to_filename.h> - -#include <kernel-features.h> +#include <shlib-compat.h> FILE * freopen (const char *filename, const char *mode, FILE *fp) { - FILE *result; + FILE *result = NULL; + char fdfilename[30]; + CHECK_FILE (fp, NULL); - if (!(fp->_flags & _IO_IS_FILEBUF)) - return NULL; + _IO_acquire_lock (fp); + /* First flush the stream (failure should be ignored). */ + _IO_SYNC (fp); + + if (!(fp->_flags & _IO_IS_FILEBUF)) + goto end; + int fd = _IO_fileno (fp); - const char *gfilename = (filename == NULL && fd >= 0 - ? fd_to_filename (fd) : filename); + const char *gfilename; + if (filename == NULL && fd >= 0) + gfilename = fd_to_filename (fd, fdfilename, sizeof fdfilename) + ? fdfilename : NULL; + else + gfilename = filename; + fp->_flags2 |= _IO_FLAGS2_NOCLOSE; #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) if (&_IO_stdin_used == NULL) @@ -101,9 +111,6 @@ freopen (const char *filename, const char *mode, FILE *fp) __close (fd); end: - if (filename == NULL) - free ((char *) gfilename); - _IO_release_lock (fp); return result; } diff --git a/libio/freopen64.c b/libio/freopen64.c index 1e56de6..28203c9 100644 --- a/libio/freopen64.c +++ b/libio/freopen64.c @@ -24,27 +24,37 @@ This exception applies to code released by its copyright holders in files containing the exception. */ -#include "libioP.h" -#include "stdio.h" +#include <stdio.h> #include <fcntl.h> #include <stdlib.h> #include <unistd.h> +#include <libioP.h> #include <fd_to_filename.h> -#include <kernel-features.h> - FILE * freopen64 (const char *filename, const char *mode, FILE *fp) { - FILE *result; + FILE *result = NULL; + char fdfilename[30]; + CHECK_FILE (fp, NULL); - if (!(fp->_flags & _IO_IS_FILEBUF)) - return NULL; + _IO_acquire_lock (fp); + /* First flush the stream (failure should be ignored). */ + _IO_SYNC (fp); + + if (!(fp->_flags & _IO_IS_FILEBUF)) + goto end; + int fd = _IO_fileno (fp); - const char *gfilename = (filename == NULL && fd >= 0 - ? fd_to_filename (fd) : filename); + const char *gfilename; + if (filename == NULL && fd >= 0) + gfilename = fd_to_filename (fd, fdfilename, sizeof fdfilename) + ? fdfilename : NULL; + else + gfilename = filename; + fp->_flags2 |= _IO_FLAGS2_NOCLOSE; _IO_file_close_it (fp); _IO_JUMPS_FILE_plus (fp) = &_IO_file_jumps; @@ -84,8 +94,6 @@ freopen64 (const char *filename, const char *mode, FILE *fp) __close (fd); end: - if (filename == NULL) - free ((char *) gfilename); _IO_release_lock (fp); return result; } diff --git a/libio/tst-memstream.h b/libio/tst-memstream.h index ffc2a65..86ea0d7 100644 --- a/libio/tst-memstream.h +++ b/libio/tst-memstream.h @@ -49,6 +49,7 @@ fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq) # define PRINTF wprintf # define FWRITE fwwrite # define FPUTC fputwc +# define FPUTS fputws # define STRCMP wcscmp # define STRLEN wcslen #else @@ -58,6 +59,7 @@ fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq) # define PRINTF printf # define FWRITE fwrite # define FPUTC fputc +# define FPUTS fputs # define STRCMP strcmp # define STRLEN strlen #endif diff --git a/libio/tst-memstream5.c b/libio/tst-memstream5.c new file mode 100644 index 0000000..7c1f37c --- /dev/null +++ b/libio/tst-memstream5.c @@ -0,0 +1,62 @@ +/* Test for open_memstream BZ #21037. + 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 "tst-memstream.h" + +static void +mcheck_abort (enum mcheck_status ev) +{ + printf ("mecheck failed with status %d\n", (int) ev); + exit (1); +} + +static int +do_test (void) +{ + mcheck_pedantic (mcheck_abort); + + /* Check if freopen proper fflush the stream. */ + { + CHAR_T old[] = W("old"); + CHAR_T *buf = old; + size_t size; + + FILE *fp = OPEN_MEMSTREAM (&buf, &size); + TEST_VERIFY_EXIT (fp != NULL); + + FPUTS (W("new"), fp); + /* The stream buffer pointer should be updated with only a fflush or + fclose. */ + TEST_VERIFY (STRCMP (buf, old) == 0); + + /* The old stream should be fflush the stream, even for an invalid + streams. */ + FILE *nfp = freopen ("invalid-file", "r", fp); + TEST_VERIFY (nfp == NULL); + + TEST_VERIFY (STRCMP (buf, W("new")) == 0); + + TEST_VERIFY (fclose (fp) == 0); + + free (buf); + } + + return 0; +} + +#include <support/test-driver.c> diff --git a/libio/tst-wmemstream5.c b/libio/tst-wmemstream5.c new file mode 100644 index 0000000..4e87289 --- /dev/null +++ b/libio/tst-wmemstream5.c @@ -0,0 +1,20 @@ +/* Test for open_wmemstream BZ #21037. + 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/>. */ + +#define TEST_WCHAR +#include <libio/tst-memstream5.c> diff --git a/sysdeps/generic/fd_to_filename.h b/sysdeps/generic/fd_to_filename.h index fe15f5d..db594f2 100644 --- a/sysdeps/generic/fd_to_filename.h +++ b/sysdeps/generic/fd_to_filename.h @@ -18,8 +18,8 @@ /* In general there is no generic way to query filename for an open file descriptor. */ -static inline const char * -fd_to_filename (int fd) +static inline bool +fd_to_filename (int fd, char *buf, size_t len) { - return NULL; + return false; } diff --git a/sysdeps/unix/sysv/linux/fd_to_filename.h b/sysdeps/unix/sysv/linux/fd_to_filename.h index 414a30f..6cd0bdd 100644 --- a/sysdeps/unix/sysv/linux/fd_to_filename.h +++ b/sysdeps/unix/sysv/linux/fd_to_filename.h @@ -16,30 +16,17 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -#include <stdlib.h> -#include <string.h> +#include <stdio.h> #include <sys/stat.h> -#include <_itoa.h> -static inline const char * -fd_to_filename (int fd) +static inline bool +fd_to_filename (int fd, char *buf, size_t len) { - char *ret = malloc (30); + __snprintf (buf, len, "/proc/self/fd/%d", fd); - if (ret != NULL) - { - struct stat64 st; - - *_fitoa_word (fd, __stpcpy (ret, "/proc/self/fd/"), 10, 0) = '\0'; - - /* We must make sure the file exists. */ - if (__lxstat64 (_STAT_VER, ret, &st) < 0) - { - /* /proc is not mounted or something else happened. Don't - return the file name. */ - free (ret); - ret = NULL; - } - } - return ret; + /* We must make sure the file exists. */ + if (__lxstat64 (_STAT_VER, buf, & (struct stat64) {}) < 0) + /* /proc is not mounted or something else happened. */ + return false; + return true; }
As POSIX states [1] a freopen call should first flush the stream as if by a call fflush. C99 (n1256) and C11 (n1570) only states the function should first close any file associated with the specific stream. Although current implementation only follow C specification, current BSD and other libc implementation (musl) are in sync with POSIX and fflush the stream. This patch change freopen{64} to fflush the stream before actually reopening it (or returning if the stream does not support reopen). It also changes the Linux implementation to avoid a dynamic allocation on 'fd_to_filename'. Checked on x86_64-linux-gnu. From Florian's comment, this is intended for 2.27. This patch should be applied on top on 'libio: Fix open_memstream fflush (NULL) (BZ#21735)' [2]. [BZ #21037] * libio/freopen.c (freopen): Sync stream before reopen and adjust to new fd_to_filename interface. * libio/freopen64.c (freopen64): Likewise. * sysdeps/generic/fd_to_filename.h (fd_to_filename): Change signature. * sysdeps/unix/sysv/linux/fd_to_filename.h (fd_to_filename): Likewise and remove internal dynamic allocation. * libio/Makefile (tests): Add tst-memstream5 and tst-wmemstream5. * libio/tst-memstream.h (FPUTS): New macro. * libio/tst-memstream4.c: New file. * libio/tst-wmemstream4.c: Likewise. [1] http://pubs.opengroup.org/onlinepubs/9699919799/ [2] https://sourceware.org/ml/libc-alpha/2017-07/msg00548.html --- ChangeLog | 14 ++++++++ libio/Makefile | 3 +- libio/freopen.c | 33 ++++++++++------- libio/freopen64.c | 30 ++++++++++------ libio/tst-memstream.h | 2 ++ libio/tst-memstream5.c | 62 ++++++++++++++++++++++++++++++++ libio/tst-wmemstream5.c | 20 +++++++++++ sysdeps/generic/fd_to_filename.h | 6 ++-- sysdeps/unix/sysv/linux/fd_to_filename.h | 31 +++++----------- 9 files changed, 151 insertions(+), 50 deletions(-) create mode 100644 libio/tst-memstream5.c create mode 100644 libio/tst-wmemstream5.c -- 2.7.4