Re: [PATCH] Fix wide stdio

[Jakub Jelinek <jakub at redhat dot com>]

On Fri, Nov 24, 2006 at 07:17:22AM -0800, Ulrich Drepper wrote:
> Jakub Jelinek wrote:
> >testcase from Arjan we attempt to munmap fp->_shortbuf (which is 
> >fortunately
> >not page aligned and thus the munmap just fails).
> 
> No.  We have this bug in the upstream BZ for some time and even had one 
> of the IBM people try to provide a patch.  It's not that easy.  The 
> current code is screwed up big time when it comes to wide streams and 
> setvbuf.  It's not such a trivial change.

I'd like to understand what's still screwed up big time after the patch
I posted.  For _IONBF I don't see what is broken, the _IO_UNBUFFERED
flag is set correctly in that case and _IO_buf_base is _shortbuf
while _wide_data->_IO_buf_base is _wide_data->_shortbuf (and _end
similarly).
In other cases things IMHO work just fine too, the only question is
what is the user passed buffer supposed to be used for in wide IO,
we need 2 kinds of buffers, one char buffer, one wchar_t buffer.
Currently the user buffer is used as the char buffer and uses a
default sized allocated buffer for the other cases.  I don't think
that violates POSIX, but if e.g. the user buffer is intentionally small,
then it might not be 100% what the user would expect.
Alternatives include (when not unbuffered, which is correctly handled
with the _shortbuf buffers):
- _IO_wide would allocate a wide (non-user) buffer with
  size in some correlation to the size of the narrow buffer
  (be it (_IO_buf_end - _IO_buf_base) * sizeof (wchar_t), or
  (_IO_buf_end - _IO_buf_base) / average_length_of_mb_char * sizeof (wchar_t),
  (_IO_buf_end - _IO_buf_base) or something similar
- use user provided buffer as the wide buffer (after aligning it if needed)
  and allocate a new narrow buffer, either with a size in some correlation
  to the wide buffer size or using default size
- split the user provided buffer into two regions, use one region for the
  wide buffer and the other region for the narrow buffer
It is certainly not obvious what the right behavior is and I think neither
of the 4 alternatives (including the current one) violates the standard.

I looked at BZ#2337.
http://sources.redhat.com/bugzilla/show_bug.cgi?id=2337#c4
lists several options to fix the bug as well:

Solution 1.) in there is basically the middle alternative above with default
size narrow buffer, but in itself isn't the right fix, as e.g. the wide
buffer can still be a user buffer, while narrow buffer default allocated
and so sharing the same _IO_USER_BUF bit for it is bad.

Solution 2a.), i.e. replace fp->_flags = _IO_MAGIC|CLOSED_FILEBUF_FLAGS;
in _IO_new_file_close_it with
fp->_flags = _IO_MAGIC|CLOSED_FILEBUF_FLAGS | (fp->_flags & _IO_USER_BUF);
is certainly possible, but only if we guarantee that at any time either
both narrow and wide buffers are malloced, or are alloced by user (resp.
_shortbuf).

Solution 2b.) which has a patch provided sounds very wrong, that just
means we will leak the narrow buffer.  wdefault_finish makes sense for
wstrops (where we don't use two kinds of buffers, just the wide one).

The patch I posted had two parts, one was the addition of
_IO_FLAGS2_USER_WBUF flag and using it wherever the wide buf was meant,
the other was doing _IO_setb (fp, NULL, NULL, 0) even for wide oriented
streams in _IO_new_file_close_it.  I believe the latter is a nicer
alternative to 2a.) above and then either we need the former (USER_WBUF
flag), or we need to guarantee both buffers have the same allocation
status (this would essentially require the "user user suplied buffer for
both narrow and wide buffers").

	Jakub