This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
Hi, On Sun, Feb 03, 2019 at 11:04:48AM +0100, Florian Weimer wrote: > These variables are only used to determine if a stdio stream is > a pre-allocated stream, but it is possible to do so by comparing > a FILE * to all pre-allocated stream objects. As a result, it is > not necessary to keep those pointers in separate variables. > > Behavior with symbol interposition is unchanged because _IO_stdin_, > _IO_stdout_, _IO_stderr_ are exported, and refer to objects outside of > libc if symbol interposition or copy relocations are involved. > > 2019-02-03 Florian Weimer <fweimer@redhat.com> > > * libio/libio.h (_IO_stdin, _IO_stdout, _IO_stderr): Remove > declaration. > * libio/stdio.c (AL, AL2, _IO_stdin, _IO_stdout, _IO_stderr): > Remove definitions. > * libio/stdfiles.c: Update comment. > * libio/oldstdfiles.c (_IO_check_libio): Update comment. Do not > set _IO_stdin, _IO_stdout, _IO_stderr. > * libio/libioP.h (_IO_fake_stdiobuf): Remove unused declaration. > [SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1)] (_IO_legacy_file): New > inline function. > (_IO_deallocate_file): New inline function. > * libio/iolibio.h (_IO_vprintf): Remove definition. > * libio/iofclose.c (_IO_new_fclose): Use _IO_deallocate_file. > * libio/oldiofclose.c (_IO_old_fclose): Likewise. > * libio/iofwide.c (_IO_fwide): Use __glibc_unlikely and > _IO_legacy_file. > * libio/oldfileops.c (_IO_old_file_init_internal): Remove > __builtin_expect. Use _IO_legacy_file. Looks like _IO_legacy_file makes sense only when &_IO_stdin_used == NULL. If the check was moved inside _IO_legacy_file, then ... > diff --git a/libio/iofclose.c b/libio/iofclose.c > index 9b39a6cc4e..8a80dd0b78 100644 > --- a/libio/iofclose.c > +++ b/libio/iofclose.c > @@ -71,12 +71,7 @@ _IO_new_fclose (FILE *fp) > if (_IO_have_backup (fp)) > _IO_free_backup_area (fp); > } > - if (fp != _IO_stdin && fp != _IO_stdout && fp != _IO_stderr) > - { > - fp->_flags = 0; > - free(fp); > - } > - > + _IO_deallocate_file (fp); > return status; > } > > diff --git a/libio/iofwide.c b/libio/iofwide.c > index 6676ad5e53..247cfde3d0 100644 > --- a/libio/iofwide.c > +++ b/libio/iofwide.c > @@ -87,8 +87,7 @@ _IO_fwide (FILE *fp, int mode) > mode = mode < 0 ? -1 : (mode == 0 ? 0 : 1); > > #if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) > - if (__builtin_expect (&_IO_stdin_used == NULL, 0) > - && (fp == _IO_stdin || fp == _IO_stdout || fp == _IO_stderr)) > + if (__glibc_unlikely (&_IO_stdin_used == NULL) && _IO_legacy_file (fp)) ... this would become if (_IO_legacy_file (fp))) > /* This is for a stream in the glibc 2.0 format. */ > return -1; > #endif > diff --git a/libio/iolibio.h b/libio/iolibio.h > index 2642d71e4f..9561833655 100644 > --- a/libio/iolibio.h > +++ b/libio/iolibio.h > @@ -58,8 +58,6 @@ extern int _IO_vsscanf (const char *, const char *, __gnuc_va_list) __THROW; > == _IO_pos_BAD ? EOF : 0) > #define _IO_rewind(FILE) \ > (void) _IO_seekoff_unlocked (FILE, 0, 0, _IOS_INPUT|_IOS_OUTPUT) > -#define _IO_vprintf(FORMAT, ARGS) \ > - _IO_vfprintf (_IO_stdout, FORMAT, ARGS) > #define _IO_freopen(FILENAME, MODE, FP) \ > (_IO_file_close_it (FP), \ > _IO_file_fopen (FP, FILENAME, MODE, 1)) > diff --git a/libio/libio.h b/libio/libio.h > index d21162aab0..872574cfb7 100644 > --- a/libio/libio.h > +++ b/libio/libio.h > @@ -185,9 +185,6 @@ struct _IO_FILE_plus; > extern struct _IO_FILE_plus _IO_2_1_stdin_; > extern struct _IO_FILE_plus _IO_2_1_stdout_; > extern struct _IO_FILE_plus _IO_2_1_stderr_; > -extern FILE *_IO_stdin attribute_hidden; > -extern FILE *_IO_stdout attribute_hidden; > -extern FILE *_IO_stderr attribute_hidden; > > struct _IO_cookie_file; > > diff --git a/libio/libioP.h b/libio/libioP.h > index 8c75f15167..1c434ec3a1 100644 > --- a/libio/libioP.h > +++ b/libio/libioP.h > @@ -817,7 +817,35 @@ extern int _IO_vscanf (const char *, va_list) __THROW; > # endif > #endif > > -extern struct _IO_fake_stdiobuf _IO_stdin_buf, _IO_stdout_buf, _IO_stderr_buf; > +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) > +/* See oldstdfiles.c. These are the old stream variables. */ > +extern struct _IO_FILE_plus _IO_stdin_; > +extern struct _IO_FILE_plus _IO_stdout_; > +extern struct _IO_FILE_plus _IO_stderr_; > + > +static inline bool > +_IO_legacy_file (FILE *fp) > +{ > + return fp == (FILE *) &_IO_stdin_ || fp == (FILE *) &_IO_stdout_ > + || fp == (FILE *) &_IO_stderr_; > +} > +#endif > + > +/* Deallocate a stream if it is heap-allocated. Preallocated > + stdin/stdout/stderr streams are not deallocated. */ > +static inline void > +_IO_deallocate_file (FILE *fp) > +{ > + /* The current stream variables. */ > + if (fp == (FILE *) &_IO_2_1_stdin_ || fp == (FILE *) &_IO_2_1_stdout_ > + || fp == (FILE *) &_IO_2_1_stderr_) > + return; > +#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_1) > + if (_IO_legacy_file (fp)) ... this would save us from 3 extra checks for _IO_stdin_, _IO_stdout_, and _IO_stderr_, and ... > + return; > +#endif > + free (fp); > +} > > #ifdef IO_DEBUG > # define CHECK_FILE(FILE, RET) do { \ > diff --git a/libio/oldfileops.c b/libio/oldfileops.c > index 4d6c5e3fe7..10f2205e8a 100644 > --- a/libio/oldfileops.c > +++ b/libio/oldfileops.c > @@ -109,10 +109,7 @@ _IO_old_file_init_internal (struct _IO_FILE_plus *fp) > - (int) sizeof (struct _IO_FILE_complete)); > fp->file._fileno = -1; > > - if (__builtin_expect (&_IO_stdin_used != NULL, 1) > - || (fp != (struct _IO_FILE_plus *) _IO_stdin > - && fp != (struct _IO_FILE_plus *) _IO_stdout > - && fp != (struct _IO_FILE_plus *) _IO_stderr)) > + if (&_IO_stdin_used != NULL && !_IO_legacy_file ((FILE *) fp)) ... and this would become if (!_IO_legacy_file ((FILE *) fp)) Overall a bit cleaner and a bit less error-prone. -- ldv
Attachment:
signature.asc
Description: PGP signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |