Re: [PATCH] libio: Eliminate _IO_stdin, _IO_stdout, _IO_stderr

["Gabriel F. T. Gomes" <gabriel at inconstante dot eti dot br>]

On Mon, Feb 18 2019, Florian Weimer wrote:
> 
> So in a new program (which defines _IO_stdin_used), we would assume the
> _mode member is present.  In an old program (no _IO_stdin_used), we look
> at _IO_stdin etc., which will have been initialized to point to
> _IO_stdin_ etc.  If fp does not point to one of these objects, we write
> to _mode.
>
> ...
>
> libio is somewhat inconsistent in the way it checks for legacy file
> handles.  freopen uses the absence of _IO_stdin_used.  _IO_old_fopen
> (the old implementation of fopen) does *not* look at _IO_stdin_used and
> always allocates a large struct, which includes the _mode member:
> 
> $ gdb libio/oldiofopen.os
> …
> (gdb) print sizeof(struct _IO_FILE_complete_plus)
> $1 = 152
> 
> Compare this to:
> 
> $ gdb libio/oldstdfiles.os
> …
> (gdb) print sizeof(_IO_stdin_)
> $1 = 80
> 
> Presumably, this makes the assignment to _mode safe in all cases where
> the object is not one of the pre-allocated _IO_stdin_ (etc.) objects.
> It is not safe if a shared object brings its own statically allocated
> FILE object (no matter how it is named) and the shared object is loaded
> into a new application (which defines _IO_stdin_used).  This object will
> not have a _mode member, yet, _IO_old_file_init_internal tries to assign
> to it.

Thanks for the detailed explanations.

> libio: Eliminate _IO_stdin, _IO_stdout, _IO_stderr
> 
> 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.  (The
> removed variables _IO_stdin, _IO_stdout, _IO_stderr were not exported,
> of course.)

OK.

> -  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))

OK.

This version looks good to me. Thanks!