From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27601 invoked by alias); 27 Nov 2006 14:05:31 -0000 Received: (qmail 27583 invoked by uid 22791); 27 Nov 2006 14:05:31 -0000 X-Spam-Check-By: sourceware.org Received: from sunsite.ms.mff.cuni.cz (HELO sunsite.mff.cuni.cz) (195.113.15.26) by sourceware.org (qpsmtpd/0.31) with ESMTP; Mon, 27 Nov 2006 14:05:21 +0000 Received: from sunsite.mff.cuni.cz (sunsite.mff.cuni.cz [127.0.0.1]) by sunsite.mff.cuni.cz (8.13.1/8.13.1) with ESMTP id kARE58aw029142; Mon, 27 Nov 2006 15:05:08 +0100 Received: (from jj@localhost) by sunsite.mff.cuni.cz (8.13.1/8.13.1/Submit) id kARE57jk029137; Mon, 27 Nov 2006 15:05:07 +0100 Date: Mon, 27 Nov 2006 14:05:00 -0000 From: Jakub Jelinek To: Ulrich Drepper Cc: Arjan van de Ven , Glibc hackers Subject: Re: [PATCH] Fix wide stdio Message-ID: <20061127140506.GN3849@sunsite.mff.cuni.cz> Reply-To: Jakub Jelinek References: <20061124130448.GK3849@sunsite.mff.cuni.cz> <45670D02.6010402@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <45670D02.6010402@redhat.com> User-Agent: Mutt/1.4.1i Mailing-List: contact libc-hacker-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-hacker-owner@sourceware.org X-SW-Source: 2006-11/txt/msg00019.txt.bz2 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