Fix BZ#16374 -- don't use mmap for FILE buffers
Commit Message
Greetings,
I believe we've reached a consensus in
https://sourceware.org/ml/libc-alpha/2015-02/msg00010.html -- using
mmap to allocate FILE buffers is really not a good idea.
Attached patch replaces mmap()s with calloc()s. Doing this revealed a
few cleanup bugs: wide stream and input-only buffers were not cleaned
up under some conditions.
Tested on Linux/x86_64 with no new failures.
2015-02-16 Paul Pluzhnikov <ppluzhnikov@google.com>
[BZ #16374]
* NEWS: mention 16374
* libio/filedoalloc.c (_IO_file_doallocate,
_IO_default_doallocate): Use calloc instead of ALLOC_BUF
(_IO_setb, _IO_default_finish): Use free instead of FREE_BUF
(_IO_default_finish): Likewise. Fix cleanup bugs.
(libc_freeres_fn): Clean up wide stream buffers.
* libio/libio.h (struct _IO_FILE_complete): Delete
_freeres_size, add _freeres_wbuf
* libio/libioP.h (ROUND_TO_PAGE, FREE_BUF, ALLOC_BUF,
ALLOC_WBUF): Delete
* libio/wfiledoalloc.c (_IO_wfile_doallocate): Use calloc
instead of ALLOC_WBUF
* libio/wgenops.c (_IO_wdefault_doallocate): Likewise
(_IO_wsetb, _IO_wdefault_finish): Use free instead of FREE_BUF
--
Paul Pluzhnikov
Comments
On Mon, 16 Feb 2015, Paul Pluzhnikov wrote:
> Attached patch replaces mmap()s with calloc()s. Doing this revealed a
> few cleanup bugs: wide stream and input-only buffers were not cleaned
> up under some conditions.
Were those bugs user-visible in releases (memory leaks should be
considered user-visible)? If so, there should be a separate bug filed in
Bugzilla for them. And it is possible to test for those bugs in the
testsuite (if memory leaks, maybe with an mtrace test - we have plenty of
those already)?
> 2015-02-16 Paul Pluzhnikov <ppluzhnikov@google.com>
>
> [BZ #16374]
> * NEWS: mention 16374
I think you mean 16734 not 16374.
On Mon, Feb 16, 2015 at 10:13 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> Were those bugs user-visible in releases (memory leaks should be
> considered user-visible)?
The straight-forward application of s/ALLOC_BUF/calloc/ and
s/FREE_BUF/free/ resulted in 3 existing tests failing mtrace.
I believe they all missed __libc_freeres cleanups, so not really a
user-visible leak in traditional sense.
> I think you mean 16734 not 16374.
Yes, I did. Sorry about that.
On 02/16/2015 01:06 PM, Paul Pluzhnikov wrote:
> Greetings,
>
> I believe we've reached a consensus in
> https://sourceware.org/ml/libc-alpha/2015-02/msg00010.html -- using
> mmap to allocate FILE buffers is really not a good idea.
I agree. Using mmap is nothing bug trouble here. If the general purpose
allocator fragments, then we need to enchance it to avoid fragmentation.
> Attached patch replaces mmap()s with calloc()s. Doing this revealed a
> few cleanup bugs: wide stream and input-only buffers were not cleaned
> up under some conditions.
>
> Tested on Linux/x86_64 with no new failures.
This patch is not OK as-is, and needs to be split into multiple patches.
One patch for the calloc/free changes, and another with much much much
more detail for the semantic changes that fix input-only unclean buffers.
> 2015-02-16 Paul Pluzhnikov <ppluzhnikov@google.com>
>
> [BZ #16374]
> * NEWS: mention 16374
> * libio/filedoalloc.c (_IO_file_doallocate,
> _IO_default_doallocate): Use calloc instead of ALLOC_BUF
> (_IO_setb, _IO_default_finish): Use free instead of FREE_BUF
> (_IO_default_finish): Likewise. Fix cleanup bugs.
> (libc_freeres_fn): Clean up wide stream buffers.
> * libio/libio.h (struct _IO_FILE_complete): Delete
> _freeres_size, add _freeres_wbuf
> * libio/libioP.h (ROUND_TO_PAGE, FREE_BUF, ALLOC_BUF,
> ALLOC_WBUF): Delete
> * libio/wfiledoalloc.c (_IO_wfile_doallocate): Use calloc
> instead of ALLOC_WBUF
> * libio/wgenops.c (_IO_wdefault_doallocate): Likewise
> (_IO_wsetb, _IO_wdefault_finish): Use free instead of FREE_BUF
>
>
> --
> Paul Pluzhnikov
>
>
> bz16734.patch5.txt
>
>
> diff --git a/NEWS b/NEWS
> index 781f7a7..64a842b 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -9,8 +9,8 @@ Version 2.22
>
> * The following bugs are resolved with this release:
>
> - 4719, 15467, 15790, 16560, 17569, 17792, 17912, 17932, 17944, 17949,
> - 17964, 17965, 17967, 17969.
> + 4719, 15467, 15790, 16374, 16560, 17569, 17792, 17912, 17932, 17944,
OK.
> + 17949, 17964, 17965, 17967, 17969.
>
> Version 2.21
>
> diff --git a/libio/filedoalloc.c b/libio/filedoalloc.c
> index 918a24a..4be1b83 100644
> --- a/libio/filedoalloc.c
> +++ b/libio/filedoalloc.c
> @@ -125,7 +125,10 @@ _IO_file_doallocate (fp)
> size = st.st_blksize;
> #endif
> }
> - ALLOC_BUF (p, size, EOF);
> +
> + p = calloc (size, 1);
> + if (p == NULL) return EOF;
GNU-style please two lines e.g.
if (p == NULL)
return EOF;
Otherwise OK.
> +
> _IO_setb (fp, p, p + size, 1);
> return 1;
> }
> diff --git a/libio/genops.c b/libio/genops.c
> index 6612997..c787842 100644
> --- a/libio/genops.c
> +++ b/libio/genops.c
> @@ -398,7 +398,7 @@ _IO_setb (f, b, eb, a)
> int a;
> {
> if (f->_IO_buf_base && !(f->_flags & _IO_USER_BUF))
> - FREE_BUF (f->_IO_buf_base, _IO_blen (f));
> + free (f->_IO_buf_base);
OK.
> f->_IO_buf_base = b;
> f->_IO_buf_end = eb;
> if (a)
> @@ -587,7 +587,9 @@ _IO_default_doallocate (fp)
> {
> char *buf;
>
> - ALLOC_BUF (buf, _IO_BUFSIZ, EOF);
> + buf = calloc (_IO_BUFSIZ, 1);
> + if (buf == NULL) return EOF;
GNU-style please, two lines.
Otherwise OK.
> +
> _IO_setb (fp, buf, buf+_IO_BUFSIZ, 1);
> return 1;
> }
> @@ -687,7 +689,7 @@ _IO_default_finish (fp, dummy)
> struct _IO_marker *mark;
> if (fp->_IO_buf_base && !(fp->_flags & _IO_USER_BUF))
> {
> - FREE_BUF (fp->_IO_buf_base, _IO_blen (fp));
> + free (fp->_IO_buf_base);
OK.
> fp->_IO_buf_base = fp->_IO_buf_end = NULL;
> }
>
> @@ -950,8 +952,6 @@ _IO_unbuffer_write (void)
> for (fp = (_IO_FILE *) _IO_list_all; fp; fp = fp->_chain)
> {
> if (! (fp->_flags & _IO_UNBUFFERED)
> - && (! (fp->_flags & _IO_NO_WRITES)
> - || (fp->_flags & _IO_IS_APPENDING))
You need detailed rationale for this change.
Siddhesh made one change in libio and we had to review it
5 or 6 times to get it correct, and even then we had users
find bugs that we had to fix.
Please provide as much detail as you can about this.
> /* Iff stream is un-orientated, it wasn't used. */
> && fp->_mode != 0)
> {
> @@ -967,14 +967,30 @@ _IO_unbuffer_write (void)
> __sched_yield ();
> #endif
>
> - if (! dealloc_buffers && !(fp->_flags & _IO_USER_BUF))
> + if (!dealloc_buffers)
> {
> - fp->_flags |= _IO_USER_BUF;
> -
> - fp->_freeres_list = freeres_list;
> - freeres_list = fp;
> - fp->_freeres_buf = fp->_IO_buf_base;
> - fp->_freeres_size = _IO_blen (fp);
> + const bool need_freeres = !(fp->_flags & _IO_USER_BUF)
> + || (!(fp->_flags2 & _IO_FLAGS2_USER_WBUF) && fp->_mode > 0);
> +
> + if (need_freeres)
> + {
> + fp->_freeres_buf = fp->_freeres_wbuf = NULL;
> + fp->_freeres_list = freeres_list;
> + freeres_list = fp;
> +
> + if (!(fp->_flags & _IO_USER_BUF))
> + {
> + fp->_flags |= _IO_USER_BUF;
> + fp->_freeres_buf = fp->_IO_buf_base;
> + }
> +
> + if (fp->_mode > 0
> + && !(fp->_flags2 & _IO_FLAGS2_USER_WBUF))
> + {
> + fp->_flags2 |= _IO_FLAGS2_USER_WBUF;
> + fp->_freeres_wbuf = fp->_wide_data->_IO_buf_base;
> + }
> + }
Similarly.
> }
>
> _IO_SETBUF (fp, NULL, 0);
> @@ -998,7 +1014,8 @@ libc_freeres_fn (buffer_free)
>
> while (freeres_list != NULL)
> {
> - FREE_BUF (freeres_list->_freeres_buf, freeres_list->_freeres_size);
> + free (freeres_list->_freeres_buf);
> + free (freeres_list->_freeres_wbuf);
OK.
>
> freeres_list = freeres_list->_freeres_list;
> }
> diff --git a/libio/libio.h b/libio/libio.h
> index 9ff1fb0..fd139d3 100644
> --- a/libio/libio.h
> +++ b/libio/libio.h
> @@ -297,17 +297,17 @@ struct _IO_FILE_complete
> struct _IO_wide_data *_wide_data;
> struct _IO_FILE *_freeres_list;
> void *_freeres_buf;
> - size_t _freeres_size;
> + void *_freeres_wbuf;
> # else
> void *__pad1;
> void *__pad2;
> void *__pad3;
> void *__pad4;
> - size_t __pad5;
> + void *__pad5;
> # endif
> int _mode;
> /* Make sure we don't get into trouble again. */
> - char _unused2[15 * sizeof (int) - 4 * sizeof (void *) - sizeof (size_t)];
> + char _unused2[15 * sizeof (int) - 5 * sizeof (void *)];
Not OK right now. This will go with the other patch.
> #endif
> };
>
> diff --git a/libio/libioP.h b/libio/libioP.h
> index d8604ca..bee9eaa 100644
> --- a/libio/libioP.h
> +++ b/libio/libioP.h
> @@ -746,45 +746,6 @@ extern _IO_off64_t _IO_seekpos_unlocked (_IO_FILE *, _IO_off64_t, int)
> # define ftruncate __ftruncate
> # endif
>
> -# define ROUND_TO_PAGE(_S) \
> - (((_S) + EXEC_PAGESIZE - 1) & ~(EXEC_PAGESIZE - 1))
> -
> -# define FREE_BUF(_B, _S) \
> - munmap ((_B), ROUND_TO_PAGE (_S))
> -# define ALLOC_BUF(_B, _S, _R) \
> - do { \
> - (_B) = (char *) mmap (0, ROUND_TO_PAGE (_S), \
> - PROT_READ | PROT_WRITE, \
> - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); \
> - if ((_B) == (char *) MAP_FAILED) \
> - return (_R); \
> - } while (0)
> -# define ALLOC_WBUF(_B, _S, _R) \
> - do { \
> - (_B) = (wchar_t *) mmap (0, ROUND_TO_PAGE (_S), \
> - PROT_READ | PROT_WRITE, \
> - MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); \
> - if ((_B) == (wchar_t *) MAP_FAILED) \
> - return (_R); \
> - } while (0)
> -
> -#else /* _G_HAVE_MMAP */
> -
> -# define FREE_BUF(_B, _S) \
> - free(_B)
> -# define ALLOC_BUF(_B, _S, _R) \
> - do { \
> - (_B) = (char*)malloc(_S); \
> - if ((_B) == NULL) \
> - return (_R); \
> - } while (0)
> -# define ALLOC_WBUF(_B, _S, _R) \
> - do { \
> - (_B) = (wchar_t *)malloc(_S); \
> - if ((_B) == NULL) \
> - return (_R); \
> - } while (0)
> -
OK.
> #endif /* _G_HAVE_MMAP */
>
> #ifndef OS_FSTAT
> diff --git a/libio/wfiledoalloc.c b/libio/wfiledoalloc.c
> index 12425fd..9c7fbf6 100644
> --- a/libio/wfiledoalloc.c
> +++ b/libio/wfiledoalloc.c
> @@ -95,7 +95,10 @@ _IO_wfile_doallocate (fp)
> size = fp->_IO_buf_end - fp->_IO_buf_base;
> if ((fp->_flags & _IO_USER_BUF))
> size = (size + sizeof (wchar_t) - 1) / sizeof (wchar_t);
> - ALLOC_WBUF (p, size * sizeof (wchar_t), EOF);
> +
> + p = calloc (size, sizeof (wchar_t));
> + if (p == NULL) return EOF;
OK. GNU-Style please.
> +
> _IO_wsetb (fp, p, p + size, 1);
> return 1;
> }
> diff --git a/libio/wgenops.c b/libio/wgenops.c
> index 69f3b95..afebee5 100644
> --- a/libio/wgenops.c
> +++ b/libio/wgenops.c
> @@ -111,7 +111,7 @@ _IO_wsetb (f, b, eb, a)
> int a;
> {
> if (f->_wide_data->_IO_buf_base && !(f->_flags2 & _IO_FLAGS2_USER_WBUF))
> - FREE_BUF (f->_wide_data->_IO_buf_base, _IO_wblen (f) * sizeof (wchar_t));
> + free (f->_wide_data->_IO_buf_base);
OK.
> f->_wide_data->_IO_buf_base = b;
> f->_wide_data->_IO_buf_end = eb;
> if (a)
> @@ -195,8 +195,7 @@ _IO_wdefault_finish (fp, dummy)
> struct _IO_marker *mark;
> if (fp->_wide_data->_IO_buf_base && !(fp->_flags2 & _IO_FLAGS2_USER_WBUF))
> {
> - FREE_BUF (fp->_wide_data->_IO_buf_base,
> - _IO_wblen (fp) * sizeof (wchar_t));
> + free (fp->_wide_data->_IO_buf_base);
OK.
> fp->_wide_data->_IO_buf_base = fp->_wide_data->_IO_buf_end = NULL;
> }
>
> @@ -426,7 +425,9 @@ _IO_wdefault_doallocate (fp)
> {
> wchar_t *buf;
>
> - ALLOC_WBUF (buf, _IO_BUFSIZ, EOF);
> + buf = calloc (_IO_BUFSIZ, 1);
> + if (buf == NULL) return EOF;
OK. GNU-style please.
> +
> _IO_wsetb (fp, buf, buf + _IO_BUFSIZ, 1);
> return 1;
> }
Cheers,
Carlos.
On 02/16/2015 07:06 PM, Paul Pluzhnikov wrote:
> Greetings,
>
> I believe we've reached a consensus in
> https://sourceware.org/ml/libc-alpha/2015-02/msg00010.html -- using
> mmap to allocate FILE buffers is really not a good idea.
>
> Attached patch replaces mmap()s with calloc()s.
Why is calloc needed? The implementation needs to keep track of the
active buffer range anyway, and the old fallback code had just a malloc
AFAICS.
On Mon, Feb 16, 2015 at 10:28 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> This patch is not OK as-is, and needs to be split into multiple patches.
>
> One patch for the calloc/free changes, and another with much much much
> more detail for the semantic changes that fix input-only unclean buffers.
The first patch will break 3 (mtrace) tests, and the second will then
fix them. Is that ok?
On Mon, Feb 16, 2015 at 10:32 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> Attached patch replaces mmap()s with calloc()s.
>
> Why is calloc needed?
In previous discussion it was mentioned that something was expecting
the buffers to be zero'ed out, but you are right -- fallback used
malloc and calloc shouldn't be necessary (this should nicely speed
things up on systems with large pages :)
I've retested with s/calloc/malloc/ and found no failures.
On 02/16/2015 01:51 PM, Paul Pluzhnikov wrote:
> On Mon, Feb 16, 2015 at 10:28 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>
>> This patch is not OK as-is, and needs to be split into multiple patches.
>>
>> One patch for the calloc/free changes, and another with much much much
>> more detail for the semantic changes that fix input-only unclean buffers.
>
> The first patch will break 3 (mtrace) tests, and the second will then
> fix them. Is that ok?
Yes, post them as 0/2 (descriptions), 1/2 (conversion), and 2/2 (bug fix).
> On Mon, Feb 16, 2015 at 10:32 AM, Florian Weimer <fweimer@redhat.com> wrote:
>
>>> Attached patch replaces mmap()s with calloc()s.
>>
>> Why is calloc needed?
>
> In previous discussion it was mentioned that something was expecting
> the buffers to be zero'ed out, but you are right -- fallback used
> malloc and calloc shouldn't be necessary (this should nicely speed
> things up on systems with large pages :)
Correct.
> I've retested with s/calloc/malloc/ and found no failures.
>
Cheers,
Carlos.
On 02/16/2015 07:51 PM, Paul Pluzhnikov wrote:
> On Mon, Feb 16, 2015 at 10:32 AM, Florian Weimer <fweimer@redhat.com> wrote:
>
>>> Attached patch replaces mmap()s with calloc()s.
>>
>> Why is calloc needed?
>
> In previous discussion it was mentioned that something was expecting
> the buffers to be zero'ed out, but you are right -- fallback used
> malloc and calloc shouldn't be necessary (this should nicely speed
> things up on systems with large pages :)
I think it will still be a net win, but fopen/fclose pairs might now run
into the issue discussed in the ‘Reduce worst-case behaviour with
madvise and refault overhead’ thread. :-P
Florian Weimer <fweimer@redhat.com> writes:
> I think it will still be a net win, but fopen/fclose pairs might now run
> into the issue discussed in the ‘Reduce worst-case behaviour with
> madvise and refault overhead’ thread. :-P
mmap/munmap should have the same cost as madvice.
Andreas.
On Mon, Feb 16, 2015 at 12:10 PM, Florian Weimer <fweimer@redhat.com> wrote:
> I think it will still be a net win, but fopen/fclose pairs might now run
> into the issue discussed in the ‘Reduce worst-case behaviour with
> madvise and refault overhead’ thread. :-P
Thanks for pointing out that thread.
I don't believe we'll run into that here: AFAICT the mmap threshold is
DEFAULT_MMAP_THRESHOLD_MIN currently at 128KB, while the buffers we
are malloc()ing here are _IO_BUFSIZ of 8KB.
On Mon, 16 Feb 2015, Carlos O'Donell wrote:
> On 02/16/2015 01:51 PM, Paul Pluzhnikov wrote:
> > On Mon, Feb 16, 2015 at 10:28 AM, Carlos O'Donell <carlos@redhat.com> wrote:
> >
> >> This patch is not OK as-is, and needs to be split into multiple patches.
> >>
> >> One patch for the calloc/free changes, and another with much much much
> >> more detail for the semantic changes that fix input-only unclean buffers.
> >
> > The first patch will break 3 (mtrace) tests, and the second will then
> > fix them. Is that ok?
>
> Yes, post them as 0/2 (descriptions), 1/2 (conversion), and 2/2 (bug fix).
No, we want bisectability and should seek to avoid any commits on master
that introduce test failures.
If the bug fixes can go first (even if the bugs being fixed are latent at
present), then do them before the conversion, not after. If they can't go
first then they may need to go in the same patch as the conversion.
(If there weren't existing tests involved then splitting in your order
would work, with patch 2/2 being the one adding the new tests.)
On 02/16/2015 05:06 PM, Joseph Myers wrote:
> On Mon, 16 Feb 2015, Carlos O'Donell wrote:
>
>> On 02/16/2015 01:51 PM, Paul Pluzhnikov wrote:
>>> On Mon, Feb 16, 2015 at 10:28 AM, Carlos O'Donell <carlos@redhat.com> wrote:
>>>
>>>> This patch is not OK as-is, and needs to be split into multiple patches.
>>>>
>>>> One patch for the calloc/free changes, and another with much much much
>>>> more detail for the semantic changes that fix input-only unclean buffers.
>>>
>>> The first patch will break 3 (mtrace) tests, and the second will then
>>> fix them. Is that ok?
>>
>> Yes, post them as 0/2 (descriptions), 1/2 (conversion), and 2/2 (bug fix).
>
> No, we want bisectability and should seek to avoid any commits on master
> that introduce test failures.
I agree, but that doesn't have to impact how you post the patches, only
how you commit them. Paul can ask for both to be committed as one commit,
but split them for review. In patchwork he can make a group out of them
and we can make it a habit to always commit a group as a single commit.
Similarly for patches that apply on top of eachother, it's obvious that
intermediate results likely won't build, so you have to commit them as
a single group of patches in one commit.
For the purposes of review I have *some* idea of what is and is not part
of the fix, but Paul needs to make that explicit by splitting the patches
(but not necessarily the commits).
> If the bug fixes can go first (even if the bugs being fixed are latent at
> present), then do them before the conversion, not after. If they can't go
> first then they may need to go in the same patch as the conversion.
Agreed.
> (If there weren't existing tests involved then splitting in your order
> would work, with patch 2/2 being the one adding the new tests.)
Cheers,
Carlos.
"Carlos O'Donell" <carlos@redhat.com> writes:
> I agree, but that doesn't have to impact how you post the patches, only
> how you commit them. Paul can ask for both to be committed as one commit,
> but split them for review.
It doesn't make sense to squash the commits again if they are already
suitably split.
Andreas.
On 02/17/2015 10:29 AM, Andreas Schwab wrote:
> "Carlos O'Donell" <carlos@redhat.com> writes:
>
>> I agree, but that doesn't have to impact how you post the patches, only
>> how you commit them. Paul can ask for both to be committed as one commit,
>> but split them for review.
>
> It doesn't make sense to squash the commits again if they are already
> suitably split.
I agree, but only if they are *suitably* split. That is to say they have to
fix the bug before making the change, and that may or may not be easy,
I don't know.
c.
@@ -9,8 +9,8 @@ Version 2.22
* The following bugs are resolved with this release:
- 4719, 15467, 15790, 16560, 17569, 17792, 17912, 17932, 17944, 17949,
- 17964, 17965, 17967, 17969.
+ 4719, 15467, 15790, 16374, 16560, 17569, 17792, 17912, 17932, 17944,
+ 17949, 17964, 17965, 17967, 17969.
Version 2.21
@@ -125,7 +125,10 @@ _IO_file_doallocate (fp)
size = st.st_blksize;
#endif
}
- ALLOC_BUF (p, size, EOF);
+
+ p = calloc (size, 1);
+ if (p == NULL) return EOF;
+
_IO_setb (fp, p, p + size, 1);
return 1;
}
@@ -398,7 +398,7 @@ _IO_setb (f, b, eb, a)
int a;
{
if (f->_IO_buf_base && !(f->_flags & _IO_USER_BUF))
- FREE_BUF (f->_IO_buf_base, _IO_blen (f));
+ free (f->_IO_buf_base);
f->_IO_buf_base = b;
f->_IO_buf_end = eb;
if (a)
@@ -587,7 +587,9 @@ _IO_default_doallocate (fp)
{
char *buf;
- ALLOC_BUF (buf, _IO_BUFSIZ, EOF);
+ buf = calloc (_IO_BUFSIZ, 1);
+ if (buf == NULL) return EOF;
+
_IO_setb (fp, buf, buf+_IO_BUFSIZ, 1);
return 1;
}
@@ -687,7 +689,7 @@ _IO_default_finish (fp, dummy)
struct _IO_marker *mark;
if (fp->_IO_buf_base && !(fp->_flags & _IO_USER_BUF))
{
- FREE_BUF (fp->_IO_buf_base, _IO_blen (fp));
+ free (fp->_IO_buf_base);
fp->_IO_buf_base = fp->_IO_buf_end = NULL;
}
@@ -950,8 +952,6 @@ _IO_unbuffer_write (void)
for (fp = (_IO_FILE *) _IO_list_all; fp; fp = fp->_chain)
{
if (! (fp->_flags & _IO_UNBUFFERED)
- && (! (fp->_flags & _IO_NO_WRITES)
- || (fp->_flags & _IO_IS_APPENDING))
/* Iff stream is un-orientated, it wasn't used. */
&& fp->_mode != 0)
{
@@ -967,14 +967,30 @@ _IO_unbuffer_write (void)
__sched_yield ();
#endif
- if (! dealloc_buffers && !(fp->_flags & _IO_USER_BUF))
+ if (!dealloc_buffers)
{
- fp->_flags |= _IO_USER_BUF;
-
- fp->_freeres_list = freeres_list;
- freeres_list = fp;
- fp->_freeres_buf = fp->_IO_buf_base;
- fp->_freeres_size = _IO_blen (fp);
+ const bool need_freeres = !(fp->_flags & _IO_USER_BUF)
+ || (!(fp->_flags2 & _IO_FLAGS2_USER_WBUF) && fp->_mode > 0);
+
+ if (need_freeres)
+ {
+ fp->_freeres_buf = fp->_freeres_wbuf = NULL;
+ fp->_freeres_list = freeres_list;
+ freeres_list = fp;
+
+ if (!(fp->_flags & _IO_USER_BUF))
+ {
+ fp->_flags |= _IO_USER_BUF;
+ fp->_freeres_buf = fp->_IO_buf_base;
+ }
+
+ if (fp->_mode > 0
+ && !(fp->_flags2 & _IO_FLAGS2_USER_WBUF))
+ {
+ fp->_flags2 |= _IO_FLAGS2_USER_WBUF;
+ fp->_freeres_wbuf = fp->_wide_data->_IO_buf_base;
+ }
+ }
}
_IO_SETBUF (fp, NULL, 0);
@@ -998,7 +1014,8 @@ libc_freeres_fn (buffer_free)
while (freeres_list != NULL)
{
- FREE_BUF (freeres_list->_freeres_buf, freeres_list->_freeres_size);
+ free (freeres_list->_freeres_buf);
+ free (freeres_list->_freeres_wbuf);
freeres_list = freeres_list->_freeres_list;
}
@@ -297,17 +297,17 @@ struct _IO_FILE_complete
struct _IO_wide_data *_wide_data;
struct _IO_FILE *_freeres_list;
void *_freeres_buf;
- size_t _freeres_size;
+ void *_freeres_wbuf;
# else
void *__pad1;
void *__pad2;
void *__pad3;
void *__pad4;
- size_t __pad5;
+ void *__pad5;
# endif
int _mode;
/* Make sure we don't get into trouble again. */
- char _unused2[15 * sizeof (int) - 4 * sizeof (void *) - sizeof (size_t)];
+ char _unused2[15 * sizeof (int) - 5 * sizeof (void *)];
#endif
};
@@ -746,45 +746,6 @@ extern _IO_off64_t _IO_seekpos_unlocked (_IO_FILE *, _IO_off64_t, int)
# define ftruncate __ftruncate
# endif
-# define ROUND_TO_PAGE(_S) \
- (((_S) + EXEC_PAGESIZE - 1) & ~(EXEC_PAGESIZE - 1))
-
-# define FREE_BUF(_B, _S) \
- munmap ((_B), ROUND_TO_PAGE (_S))
-# define ALLOC_BUF(_B, _S, _R) \
- do { \
- (_B) = (char *) mmap (0, ROUND_TO_PAGE (_S), \
- PROT_READ | PROT_WRITE, \
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); \
- if ((_B) == (char *) MAP_FAILED) \
- return (_R); \
- } while (0)
-# define ALLOC_WBUF(_B, _S, _R) \
- do { \
- (_B) = (wchar_t *) mmap (0, ROUND_TO_PAGE (_S), \
- PROT_READ | PROT_WRITE, \
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); \
- if ((_B) == (wchar_t *) MAP_FAILED) \
- return (_R); \
- } while (0)
-
-#else /* _G_HAVE_MMAP */
-
-# define FREE_BUF(_B, _S) \
- free(_B)
-# define ALLOC_BUF(_B, _S, _R) \
- do { \
- (_B) = (char*)malloc(_S); \
- if ((_B) == NULL) \
- return (_R); \
- } while (0)
-# define ALLOC_WBUF(_B, _S, _R) \
- do { \
- (_B) = (wchar_t *)malloc(_S); \
- if ((_B) == NULL) \
- return (_R); \
- } while (0)
-
#endif /* _G_HAVE_MMAP */
#ifndef OS_FSTAT
@@ -95,7 +95,10 @@ _IO_wfile_doallocate (fp)
size = fp->_IO_buf_end - fp->_IO_buf_base;
if ((fp->_flags & _IO_USER_BUF))
size = (size + sizeof (wchar_t) - 1) / sizeof (wchar_t);
- ALLOC_WBUF (p, size * sizeof (wchar_t), EOF);
+
+ p = calloc (size, sizeof (wchar_t));
+ if (p == NULL) return EOF;
+
_IO_wsetb (fp, p, p + size, 1);
return 1;
}
@@ -111,7 +111,7 @@ _IO_wsetb (f, b, eb, a)
int a;
{
if (f->_wide_data->_IO_buf_base && !(f->_flags2 & _IO_FLAGS2_USER_WBUF))
- FREE_BUF (f->_wide_data->_IO_buf_base, _IO_wblen (f) * sizeof (wchar_t));
+ free (f->_wide_data->_IO_buf_base);
f->_wide_data->_IO_buf_base = b;
f->_wide_data->_IO_buf_end = eb;
if (a)
@@ -195,8 +195,7 @@ _IO_wdefault_finish (fp, dummy)
struct _IO_marker *mark;
if (fp->_wide_data->_IO_buf_base && !(fp->_flags2 & _IO_FLAGS2_USER_WBUF))
{
- FREE_BUF (fp->_wide_data->_IO_buf_base,
- _IO_wblen (fp) * sizeof (wchar_t));
+ free (fp->_wide_data->_IO_buf_base);
fp->_wide_data->_IO_buf_base = fp->_wide_data->_IO_buf_end = NULL;
}
@@ -426,7 +425,9 @@ _IO_wdefault_doallocate (fp)
{
wchar_t *buf;
- ALLOC_WBUF (buf, _IO_BUFSIZ, EOF);
+ buf = calloc (_IO_BUFSIZ, 1);
+ if (buf == NULL) return EOF;
+
_IO_wsetb (fp, buf, buf + _IO_BUFSIZ, 1);
return 1;
}