| Message ID | 1470688986-8798-2-git-send-email-adhemerval.zanella@linaro.org |
|---|---|
| State | New |
| Headers | show |
Ping. On 08/08/2016 17:43, Adhemerval Zanella wrote: > This patch fixes how open_{w}memstream updates the returned buffer > size on a fclose/fflush operation. POSIX states [1]: > > "After a successful fflush() or fclose() [...] the variable pointed > to by sizep shall contain the smaller of the current buffer length and > the number of bytes for open_memstream(), or the number of wide characters > for open_wmemstream(), between the beginning of the buffer and the current > file position indicator." > > Current GLIBC behavior returns the seek position even if there is no previous > write operation. To correctly report the buffer size the implementation must > track both the buffer positiob and currently bytes written. However internal > _IO_write_ptr is updated on both write and seek operations. > > This patch fixes it by adding two new internal fields to keep track of both > previous and next position after a write operation. It allows the sync and > close callbacks to know if a write has occurred based on previous and > current _IO_write_ptr value. > > Tested on x86_64. > > [BZ #15298] > * libio/memstream.c (_IO_FILE_memstream): Add prevwriteptr and > seekwriteptr fields. > (_IO_mem_seekoff): New function. > (_IO_mem_jumps): Use _IO_mem_seekoff. > (__open_memstream): Initialize prevwriteptr and seekwriteptr. > (_IO_mem_sync): Update sizeloc based on written bytes instead of buffer > current position. > (_IO_mem_finish): Likewise. > * libio/wmemstream.c (_IO_FILE_wmemstream): Add prevwriteptr and > seekwriteptr fields. > (_IO_wmem_seekoff): New function. > (_IO_wmem_jumps): Use _IO_mem_seekoff. > (__open_wmemstream): Initialize prevwriteptr and seekwriteptr. > (_IO_mem_wsync): Update sizeloc based on written bytes instead of buffer > current position. > (_IO_mem_wfinish): Likewise. > * libio/tst-memstream3.c (do_test_bz15298): New function. > (do_test_bz18241): Check for expected size after a fseek followed by a > fflush. > (do_test): Call do_test_bz15298. > > [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/open_memstream.html > --- > ChangeLog | 22 +++++++++++++++ > libio/memstream.c | 52 ++++++++++++++++++++++++++++++++---- > libio/tst-memstream3.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++--- > libio/wmemstream.c | 54 ++++++++++++++++++++++++++++++++----- > 4 files changed, 185 insertions(+), 15 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index 28d9012..ea16b48 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,5 +1,27 @@ > 2016-08-05 Adhemerval Zanella <adhemerval.zanella@linaro.org> > > + [BZ #15298] > + * libio/memstream.c (_IO_FILE_memstream): Add prevwriteptr and > + seekwriteptr fields. > + (_IO_mem_seekoff): New function. > + (_IO_mem_jumps): Use _IO_mem_seekoff. > + (__open_memstream): Initialize prevwriteptr and seekwriteptr. > + (_IO_mem_sync): Update sizeloc based on written bytes instead of buffer > + current position. > + (_IO_mem_finish): Likewise. > + * libio/wmemstream.c (_IO_FILE_wmemstream): Add prevwriteptr and > + seekwriteptr fields. > + (_IO_wmem_seekoff): New function. > + (_IO_wmem_jumps): Use _IO_mem_seekoff. > + (__open_wmemstream): Initialize prevwriteptr and seekwriteptr. > + (_IO_mem_wsync): Update sizeloc based on written bytes instead of buffer > + current position. > + (_IO_mem_wfinish): Likewise. > + * libio/tst-memstream3.c (do_test_bz15298): New function. > + (do_test_bz18241): Check for expected size after a fseek followed by a > + fflush. > + (do_test): Call do_test_bz15298. > + > [BZ #18241] > [BZ #20181] > * libio/Makefile (test): Add tst-memstream3 and tst-wmemstream3. > diff --git a/libio/memstream.c b/libio/memstream.c > index f1e8d58..cb3af9a 100644 > --- a/libio/memstream.c > +++ b/libio/memstream.c > @@ -26,12 +26,15 @@ struct _IO_FILE_memstream > _IO_strfile _sf; > char **bufloc; > _IO_size_t *sizeloc; > + char *prevwriteptr; > + char *seekwriteptr; > }; > > > static int _IO_mem_sync (_IO_FILE* fp) __THROW; > static void _IO_mem_finish (_IO_FILE* fp, int) __THROW; > - > +static _IO_off64_t _IO_mem_seekoff (_IO_FILE *fp, _IO_off64_t offset, > + int dir, int mode) __THROW; > > static const struct _IO_jump_t _IO_mem_jumps libio_vtable = > { > @@ -43,7 +46,7 @@ static const struct _IO_jump_t _IO_mem_jumps libio_vtable = > JUMP_INIT (pbackfail, _IO_str_pbackfail), > JUMP_INIT (xsputn, _IO_default_xsputn), > JUMP_INIT (xsgetn, _IO_default_xsgetn), > - JUMP_INIT (seekoff, _IO_str_seekoff), > + JUMP_INIT (seekoff, _IO_mem_seekoff), > JUMP_INIT (seekpos, _IO_default_seekpos), > JUMP_INIT (setbuf, _IO_default_setbuf), > JUMP_INIT (sync, _IO_mem_sync), > @@ -95,6 +98,24 @@ __open_memstream (char **bufloc, _IO_size_t *sizeloc) > > new_f->fp.bufloc = bufloc; > new_f->fp.sizeloc = sizeloc; > + /* To correctly report the buffer size the implementation must track both > + the buffer size and currently bytes written. However _IO_write_ptr is > + updated on both write and seek operations. So to track current written > + bytes two fields are used: > + > + - prevwriteptr: track previous _IO_write_ptr before a seek operation on > + the stream. > + - seekwriteptr: track resulted _IO_write_ptr after a seek operation on > + the stream. > + > + Also, prevwriteptr is only updated iff _IO_write_ptr changed over calls > + (meaning that a write operation occured) > + > + So final buffer size is based on current _IO_write_ptr only if > + its value is different than seekwriteptr, otherwise it uses the old > + _IO_write_ptr value before seek operation (prevwriteptr). */ > + new_f->fp.prevwriteptr = new_f->fp.seekwriteptr = > + new_f->fp._sf._sbf._f._IO_write_ptr; > > return (_IO_FILE *) &new_f->fp._sf._sbf; > } > @@ -114,7 +135,9 @@ _IO_mem_sync (_IO_FILE *fp) > } > > *mp->bufloc = fp->_IO_write_base; > - *mp->sizeloc = fp->_IO_write_ptr - fp->_IO_write_base; > + char *ptr = (fp->_IO_write_ptr == mp->seekwriteptr) > + ? mp->prevwriteptr : fp->_IO_write_ptr; > + *mp->sizeloc = ptr - fp->_IO_write_base; > > return 0; > } > @@ -129,11 +152,30 @@ _IO_mem_finish (_IO_FILE *fp, int dummy) > fp->_IO_write_ptr - fp->_IO_write_base + 1); > if (*mp->bufloc != NULL) > { > - (*mp->bufloc)[fp->_IO_write_ptr - fp->_IO_write_base] = '\0'; > - *mp->sizeloc = fp->_IO_write_ptr - fp->_IO_write_base; > + size_t len; > + if (fp->_IO_write_ptr == mp->seekwriteptr) > + len = mp->prevwriteptr - fp->_IO_write_base; > + else > + { > + /* An '\0' should be appended iff a write operation ocurred. */ > + len = fp->_IO_write_ptr - fp->_IO_write_base; > + (*mp->bufloc)[len] = '\0'; > + } > + *mp->sizeloc = len; > > fp->_IO_buf_base = NULL; > } > > _IO_str_finish (fp, 0); > } > + > +static _IO_off64_t > +_IO_mem_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode) > +{ > + struct _IO_FILE_memstream *mp = (struct _IO_FILE_memstream *) fp; > + if (fp->_IO_write_ptr != mp->seekwriteptr) > + mp->prevwriteptr = fp->_IO_write_ptr; > + _IO_off64_t ret = _IO_str_seekoff (fp, offset, dir, mode); > + mp->seekwriteptr = fp->_IO_write_ptr; > + return ret; > +} > diff --git a/libio/tst-memstream3.c b/libio/tst-memstream3.c > index 34b04e5..65c95ad 100644 > --- a/libio/tst-memstream3.c > +++ b/libio/tst-memstream3.c > @@ -57,6 +57,69 @@ error_printf (int line, const char *fmt, ...) > { error_printf(__LINE__, __VA_ARGS__); return 1; } > > static int > +do_test_bz15298 (void) > +{ > + CHAR_T *buf; > + size_t size; > + size_t ret; > + > + FILE *fp = OPEN_MEMSTREAM (&buf, &size); > + if (fp == NULL) > + ERROR_RET1 ("%s failed\n", S(OPEN_MEMSTREAM)); > + > + /* Move internal position but do not write any bytes. Final size should > + be 0. */ > + if (fseek (fp, 10, SEEK_SET) == -1) > + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > + if (fseek (fp, 20, SEEK_CUR) == -1) > + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > + if (fseek (fp, 30, SEEK_CUR) == -1) > + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > + if (fflush (fp) != 0) > + ERROR_RET1 ("fflush failed (errno = %d)\n", errno); > + if (size != 0) > + ERROR_RET1 ("size != 0 (got %zu)\n", size); > + > + /* Now write some bytes and change internal position. Final size should > + be based on written bytes. */ > + if (fseek (fp, 0, SEEK_SET) == -1) > + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > + if ((ret = FWRITE (W("abc"), 1, 3, fp)) != 3) > + ERROR_RET1 ("%s failed (errno = %d)\n", S(FWRITE), errno); > + if (fseek (fp, 20, SEEK_CUR) == -1) > + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > + if (fseek (fp, 30, SEEK_CUR) == -1) > + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > + if (fflush (fp) != 0) > + ERROR_RET1 ("fflush failed (errno = %d)\n", errno); > + if (size != 3) > + ERROR_RET1 ("size != 3 (got %zu)\n", size); > + > + /* Finally set position, write some bytes and change position again. Final > + size should be based again on write position. */ > + size_t offset = 2048; > + if (fseek (fp, offset, SEEK_SET) == -1) > + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > + if ((ret = FWRITE (W("abc"), 1, 3, fp)) != 3) > + ERROR_RET1 ("%s failed (errno = %d)\n", S(FWRITE), errno); > + if (fseek (fp, 20, SEEK_CUR) == -1) > + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > + if (fseek (fp, 30, SEEK_CUR) == -1) > + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > + if (fflush (fp) != 0) > + ERROR_RET1 ("fflush failed (errno = %d)\n", errno); > + if (size != offset + 3) > + ERROR_RET1 ("size != %zu (got %zu)\n", offset + 3, size); > + > + if (fclose (fp) != 0) > + ERROR_RET1 ("fclose failed (errno = %d\n", errno); > + > + free (buf); > + > + return 0; > +} > + > +static int > do_test_bz18241 (void) > { > CHAR_T *buf; > @@ -124,15 +187,17 @@ do_test_bz20181 (void) > if (fflush (fp) != 0) > ERROR_RET1 ("fflush failed (errno = %d)\n", errno); > > - /* Avoid truncating the buffer on close. */ > + /* fseek updates the internal buffer, but open_memstream should set the > + size to smaller of the buffer size and number of bytes written. Since > + it was written just character ('z') final size should be 1. */ > if (fseek (fp, 3, SEEK_SET) != 0) > ERROR_RET1 ("fseek failed (errno = %d)\n", errno); > > if (fclose (fp) != 0) > ERROR_RET1 ("fclose failed (errno = %d\n", errno); > > - if (size != 3) > - ERROR_RET1 ("size != 3\n"); > + if (size != 1) > + ERROR_RET1 ("size != 1 (got %zu)\n", size); > > if (buf[0] != W('z') > || buf[1] != W('b') > @@ -155,6 +220,7 @@ do_test (void) > > mcheck_pedantic (mcheck_abort); > > + ret += do_test_bz15298 (); > ret += do_test_bz18241 (); > ret += do_test_bz20181 (); > > diff --git a/libio/wmemstream.c b/libio/wmemstream.c > index fd01be0..2f8f22c 100644 > --- a/libio/wmemstream.c > +++ b/libio/wmemstream.c > @@ -27,12 +27,15 @@ struct _IO_FILE_wmemstream > _IO_strfile _sf; > wchar_t **bufloc; > _IO_size_t *sizeloc; > + wchar_t *prevwriteptr; > + wchar_t *seekwriteptr; > }; > > > static int _IO_wmem_sync (_IO_FILE* fp) __THROW; > static void _IO_wmem_finish (_IO_FILE* fp, int) __THROW; > - > +static _IO_off64_t _IO_wmem_seekoff (_IO_FILE *fp, _IO_off64_t offset, > + int dir, int mode) __THROW; > > static const struct _IO_jump_t _IO_wmem_jumps libio_vtable = > { > @@ -44,7 +47,7 @@ static const struct _IO_jump_t _IO_wmem_jumps libio_vtable = > JUMP_INIT (pbackfail, (_IO_pbackfail_t) _IO_wstr_pbackfail), > JUMP_INIT (xsputn, _IO_wdefault_xsputn), > JUMP_INIT (xsgetn, _IO_wdefault_xsgetn), > - JUMP_INIT (seekoff, _IO_wstr_seekoff), > + JUMP_INIT (seekoff, _IO_wmem_seekoff), > JUMP_INIT (seekpos, _IO_default_seekpos), > JUMP_INIT (setbuf, _IO_default_setbuf), > JUMP_INIT (sync, _IO_wmem_sync), > @@ -97,6 +100,24 @@ open_wmemstream (wchar_t **bufloc, _IO_size_t *sizeloc) > > new_f->fp.bufloc = bufloc; > new_f->fp.sizeloc = sizeloc; > + /* To correctly report the buffer size the implementation must track both > + the buffer size and currently bytes written. However _IO_write_ptr is > + updated on both write and seek operations. So to track current written > + bytes two fields are used: > + > + - prevwriteptr: track previous _IO_write_ptr before a seek operation on > + the stream. > + - seekwriteptr: track resulted _IO_write_ptr after a seek operation on > + the stream. > + > + Also, prevwriteptr is only updated iff _IO_write_ptr changed over calls > + (meaning that a write operation occured) > + > + So final buffer size is based on current _IO_write_ptr only if > + its value is different than seekwriteptr, otherwise it uses the old > + _IO_write_ptr value before seek operation (prevwriteptr). */ > + new_f->fp.prevwriteptr = new_f->fp.seekwriteptr = > + new_f->fp._sf._sbf._f._wide_data->_IO_write_ptr; > > return (_IO_FILE *) &new_f->fp._sf._sbf; > } > @@ -114,8 +135,9 @@ _IO_wmem_sync (_IO_FILE *fp) > } > > *mp->bufloc = fp->_wide_data->_IO_write_base; > - *mp->sizeloc = (fp->_wide_data->_IO_write_ptr > - - fp->_wide_data->_IO_write_base); > + wchar_t *ptr = (fp->_wide_data->_IO_write_ptr == mp->seekwriteptr) > + ? mp->prevwriteptr : fp->_wide_data->_IO_write_ptr; > + *mp->sizeloc = ptr - fp->_wide_data->_IO_write_base; > > return 0; > } > @@ -132,9 +154,16 @@ _IO_wmem_finish (_IO_FILE *fp, int dummy) > * sizeof (wchar_t)); > if (*mp->bufloc != NULL) > { > - size_t len = (fp->_wide_data->_IO_write_ptr > - - fp->_wide_data->_IO_write_base); > - (*mp->bufloc)[len] = '\0'; > + size_t len; > + if (fp->_wide_data->_IO_write_ptr == mp->seekwriteptr) > + len = mp->prevwriteptr - fp->_wide_data->_IO_write_base; > + else > + { > + /* An '\0' should be appended iff a write operation ocurred. */ > + len = fp->_wide_data->_IO_write_ptr > + - fp->_wide_data->_IO_write_base; > + (*mp->bufloc)[len] = L'\0'; > + } > *mp->sizeloc = len; > > fp->_wide_data->_IO_buf_base = NULL; > @@ -142,3 +171,14 @@ _IO_wmem_finish (_IO_FILE *fp, int dummy) > > _IO_wstr_finish (fp, 0); > } > + > +static _IO_off64_t > +_IO_wmem_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode) > +{ > + struct _IO_FILE_wmemstream *mp = (struct _IO_FILE_wmemstream *) fp; > + if (fp->_wide_data->_IO_write_ptr != mp->seekwriteptr) > + mp->prevwriteptr = fp->_wide_data->_IO_write_ptr; > + _IO_off64_t ret = _IO_wstr_seekoff (fp, offset, dir, mode); > + mp->seekwriteptr = fp->_wide_data->_IO_write_ptr; > + return ret; > +} >
diff --git a/ChangeLog b/ChangeLog index 28d9012..ea16b48 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,27 @@ 2016-08-05 Adhemerval Zanella <adhemerval.zanella@linaro.org> + [BZ #15298] + * libio/memstream.c (_IO_FILE_memstream): Add prevwriteptr and + seekwriteptr fields. + (_IO_mem_seekoff): New function. + (_IO_mem_jumps): Use _IO_mem_seekoff. + (__open_memstream): Initialize prevwriteptr and seekwriteptr. + (_IO_mem_sync): Update sizeloc based on written bytes instead of buffer + current position. + (_IO_mem_finish): Likewise. + * libio/wmemstream.c (_IO_FILE_wmemstream): Add prevwriteptr and + seekwriteptr fields. + (_IO_wmem_seekoff): New function. + (_IO_wmem_jumps): Use _IO_mem_seekoff. + (__open_wmemstream): Initialize prevwriteptr and seekwriteptr. + (_IO_mem_wsync): Update sizeloc based on written bytes instead of buffer + current position. + (_IO_mem_wfinish): Likewise. + * libio/tst-memstream3.c (do_test_bz15298): New function. + (do_test_bz18241): Check for expected size after a fseek followed by a + fflush. + (do_test): Call do_test_bz15298. + [BZ #18241] [BZ #20181] * libio/Makefile (test): Add tst-memstream3 and tst-wmemstream3. diff --git a/libio/memstream.c b/libio/memstream.c index f1e8d58..cb3af9a 100644 --- a/libio/memstream.c +++ b/libio/memstream.c @@ -26,12 +26,15 @@ struct _IO_FILE_memstream _IO_strfile _sf; char **bufloc; _IO_size_t *sizeloc; + char *prevwriteptr; + char *seekwriteptr; }; static int _IO_mem_sync (_IO_FILE* fp) __THROW; static void _IO_mem_finish (_IO_FILE* fp, int) __THROW; - +static _IO_off64_t _IO_mem_seekoff (_IO_FILE *fp, _IO_off64_t offset, + int dir, int mode) __THROW; static const struct _IO_jump_t _IO_mem_jumps libio_vtable = { @@ -43,7 +46,7 @@ static const struct _IO_jump_t _IO_mem_jumps libio_vtable = JUMP_INIT (pbackfail, _IO_str_pbackfail), JUMP_INIT (xsputn, _IO_default_xsputn), JUMP_INIT (xsgetn, _IO_default_xsgetn), - JUMP_INIT (seekoff, _IO_str_seekoff), + JUMP_INIT (seekoff, _IO_mem_seekoff), JUMP_INIT (seekpos, _IO_default_seekpos), JUMP_INIT (setbuf, _IO_default_setbuf), JUMP_INIT (sync, _IO_mem_sync), @@ -95,6 +98,24 @@ __open_memstream (char **bufloc, _IO_size_t *sizeloc) new_f->fp.bufloc = bufloc; new_f->fp.sizeloc = sizeloc; + /* To correctly report the buffer size the implementation must track both + the buffer size and currently bytes written. However _IO_write_ptr is + updated on both write and seek operations. So to track current written + bytes two fields are used: + + - prevwriteptr: track previous _IO_write_ptr before a seek operation on + the stream. + - seekwriteptr: track resulted _IO_write_ptr after a seek operation on + the stream. + + Also, prevwriteptr is only updated iff _IO_write_ptr changed over calls + (meaning that a write operation occured) + + So final buffer size is based on current _IO_write_ptr only if + its value is different than seekwriteptr, otherwise it uses the old + _IO_write_ptr value before seek operation (prevwriteptr). */ + new_f->fp.prevwriteptr = new_f->fp.seekwriteptr = + new_f->fp._sf._sbf._f._IO_write_ptr; return (_IO_FILE *) &new_f->fp._sf._sbf; } @@ -114,7 +135,9 @@ _IO_mem_sync (_IO_FILE *fp) } *mp->bufloc = fp->_IO_write_base; - *mp->sizeloc = fp->_IO_write_ptr - fp->_IO_write_base; + char *ptr = (fp->_IO_write_ptr == mp->seekwriteptr) + ? mp->prevwriteptr : fp->_IO_write_ptr; + *mp->sizeloc = ptr - fp->_IO_write_base; return 0; } @@ -129,11 +152,30 @@ _IO_mem_finish (_IO_FILE *fp, int dummy) fp->_IO_write_ptr - fp->_IO_write_base + 1); if (*mp->bufloc != NULL) { - (*mp->bufloc)[fp->_IO_write_ptr - fp->_IO_write_base] = '\0'; - *mp->sizeloc = fp->_IO_write_ptr - fp->_IO_write_base; + size_t len; + if (fp->_IO_write_ptr == mp->seekwriteptr) + len = mp->prevwriteptr - fp->_IO_write_base; + else + { + /* An '\0' should be appended iff a write operation ocurred. */ + len = fp->_IO_write_ptr - fp->_IO_write_base; + (*mp->bufloc)[len] = '\0'; + } + *mp->sizeloc = len; fp->_IO_buf_base = NULL; } _IO_str_finish (fp, 0); } + +static _IO_off64_t +_IO_mem_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode) +{ + struct _IO_FILE_memstream *mp = (struct _IO_FILE_memstream *) fp; + if (fp->_IO_write_ptr != mp->seekwriteptr) + mp->prevwriteptr = fp->_IO_write_ptr; + _IO_off64_t ret = _IO_str_seekoff (fp, offset, dir, mode); + mp->seekwriteptr = fp->_IO_write_ptr; + return ret; +} diff --git a/libio/tst-memstream3.c b/libio/tst-memstream3.c index 34b04e5..65c95ad 100644 --- a/libio/tst-memstream3.c +++ b/libio/tst-memstream3.c @@ -57,6 +57,69 @@ error_printf (int line, const char *fmt, ...) { error_printf(__LINE__, __VA_ARGS__); return 1; } static int +do_test_bz15298 (void) +{ + CHAR_T *buf; + size_t size; + size_t ret; + + FILE *fp = OPEN_MEMSTREAM (&buf, &size); + if (fp == NULL) + ERROR_RET1 ("%s failed\n", S(OPEN_MEMSTREAM)); + + /* Move internal position but do not write any bytes. Final size should + be 0. */ + if (fseek (fp, 10, SEEK_SET) == -1) + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); + if (fseek (fp, 20, SEEK_CUR) == -1) + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); + if (fseek (fp, 30, SEEK_CUR) == -1) + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); + if (fflush (fp) != 0) + ERROR_RET1 ("fflush failed (errno = %d)\n", errno); + if (size != 0) + ERROR_RET1 ("size != 0 (got %zu)\n", size); + + /* Now write some bytes and change internal position. Final size should + be based on written bytes. */ + if (fseek (fp, 0, SEEK_SET) == -1) + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); + if ((ret = FWRITE (W("abc"), 1, 3, fp)) != 3) + ERROR_RET1 ("%s failed (errno = %d)\n", S(FWRITE), errno); + if (fseek (fp, 20, SEEK_CUR) == -1) + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); + if (fseek (fp, 30, SEEK_CUR) == -1) + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); + if (fflush (fp) != 0) + ERROR_RET1 ("fflush failed (errno = %d)\n", errno); + if (size != 3) + ERROR_RET1 ("size != 3 (got %zu)\n", size); + + /* Finally set position, write some bytes and change position again. Final + size should be based again on write position. */ + size_t offset = 2048; + if (fseek (fp, offset, SEEK_SET) == -1) + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); + if ((ret = FWRITE (W("abc"), 1, 3, fp)) != 3) + ERROR_RET1 ("%s failed (errno = %d)\n", S(FWRITE), errno); + if (fseek (fp, 20, SEEK_CUR) == -1) + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); + if (fseek (fp, 30, SEEK_CUR) == -1) + ERROR_RET1 ("fseek failed (errno = %d)\n", errno); + if (fflush (fp) != 0) + ERROR_RET1 ("fflush failed (errno = %d)\n", errno); + if (size != offset + 3) + ERROR_RET1 ("size != %zu (got %zu)\n", offset + 3, size); + + if (fclose (fp) != 0) + ERROR_RET1 ("fclose failed (errno = %d\n", errno); + + free (buf); + + return 0; +} + +static int do_test_bz18241 (void) { CHAR_T *buf; @@ -124,15 +187,17 @@ do_test_bz20181 (void) if (fflush (fp) != 0) ERROR_RET1 ("fflush failed (errno = %d)\n", errno); - /* Avoid truncating the buffer on close. */ + /* fseek updates the internal buffer, but open_memstream should set the + size to smaller of the buffer size and number of bytes written. Since + it was written just character ('z') final size should be 1. */ if (fseek (fp, 3, SEEK_SET) != 0) ERROR_RET1 ("fseek failed (errno = %d)\n", errno); if (fclose (fp) != 0) ERROR_RET1 ("fclose failed (errno = %d\n", errno); - if (size != 3) - ERROR_RET1 ("size != 3\n"); + if (size != 1) + ERROR_RET1 ("size != 1 (got %zu)\n", size); if (buf[0] != W('z') || buf[1] != W('b') @@ -155,6 +220,7 @@ do_test (void) mcheck_pedantic (mcheck_abort); + ret += do_test_bz15298 (); ret += do_test_bz18241 (); ret += do_test_bz20181 (); diff --git a/libio/wmemstream.c b/libio/wmemstream.c index fd01be0..2f8f22c 100644 --- a/libio/wmemstream.c +++ b/libio/wmemstream.c @@ -27,12 +27,15 @@ struct _IO_FILE_wmemstream _IO_strfile _sf; wchar_t **bufloc; _IO_size_t *sizeloc; + wchar_t *prevwriteptr; + wchar_t *seekwriteptr; }; static int _IO_wmem_sync (_IO_FILE* fp) __THROW; static void _IO_wmem_finish (_IO_FILE* fp, int) __THROW; - +static _IO_off64_t _IO_wmem_seekoff (_IO_FILE *fp, _IO_off64_t offset, + int dir, int mode) __THROW; static const struct _IO_jump_t _IO_wmem_jumps libio_vtable = { @@ -44,7 +47,7 @@ static const struct _IO_jump_t _IO_wmem_jumps libio_vtable = JUMP_INIT (pbackfail, (_IO_pbackfail_t) _IO_wstr_pbackfail), JUMP_INIT (xsputn, _IO_wdefault_xsputn), JUMP_INIT (xsgetn, _IO_wdefault_xsgetn), - JUMP_INIT (seekoff, _IO_wstr_seekoff), + JUMP_INIT (seekoff, _IO_wmem_seekoff), JUMP_INIT (seekpos, _IO_default_seekpos), JUMP_INIT (setbuf, _IO_default_setbuf), JUMP_INIT (sync, _IO_wmem_sync), @@ -97,6 +100,24 @@ open_wmemstream (wchar_t **bufloc, _IO_size_t *sizeloc) new_f->fp.bufloc = bufloc; new_f->fp.sizeloc = sizeloc; + /* To correctly report the buffer size the implementation must track both + the buffer size and currently bytes written. However _IO_write_ptr is + updated on both write and seek operations. So to track current written + bytes two fields are used: + + - prevwriteptr: track previous _IO_write_ptr before a seek operation on + the stream. + - seekwriteptr: track resulted _IO_write_ptr after a seek operation on + the stream. + + Also, prevwriteptr is only updated iff _IO_write_ptr changed over calls + (meaning that a write operation occured) + + So final buffer size is based on current _IO_write_ptr only if + its value is different than seekwriteptr, otherwise it uses the old + _IO_write_ptr value before seek operation (prevwriteptr). */ + new_f->fp.prevwriteptr = new_f->fp.seekwriteptr = + new_f->fp._sf._sbf._f._wide_data->_IO_write_ptr; return (_IO_FILE *) &new_f->fp._sf._sbf; } @@ -114,8 +135,9 @@ _IO_wmem_sync (_IO_FILE *fp) } *mp->bufloc = fp->_wide_data->_IO_write_base; - *mp->sizeloc = (fp->_wide_data->_IO_write_ptr - - fp->_wide_data->_IO_write_base); + wchar_t *ptr = (fp->_wide_data->_IO_write_ptr == mp->seekwriteptr) + ? mp->prevwriteptr : fp->_wide_data->_IO_write_ptr; + *mp->sizeloc = ptr - fp->_wide_data->_IO_write_base; return 0; } @@ -132,9 +154,16 @@ _IO_wmem_finish (_IO_FILE *fp, int dummy) * sizeof (wchar_t)); if (*mp->bufloc != NULL) { - size_t len = (fp->_wide_data->_IO_write_ptr - - fp->_wide_data->_IO_write_base); - (*mp->bufloc)[len] = '\0'; + size_t len; + if (fp->_wide_data->_IO_write_ptr == mp->seekwriteptr) + len = mp->prevwriteptr - fp->_wide_data->_IO_write_base; + else + { + /* An '\0' should be appended iff a write operation ocurred. */ + len = fp->_wide_data->_IO_write_ptr + - fp->_wide_data->_IO_write_base; + (*mp->bufloc)[len] = L'\0'; + } *mp->sizeloc = len; fp->_wide_data->_IO_buf_base = NULL; @@ -142,3 +171,14 @@ _IO_wmem_finish (_IO_FILE *fp, int dummy) _IO_wstr_finish (fp, 0); } + +static _IO_off64_t +_IO_wmem_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode) +{ + struct _IO_FILE_wmemstream *mp = (struct _IO_FILE_wmemstream *) fp; + if (fp->_wide_data->_IO_write_ptr != mp->seekwriteptr) + mp->prevwriteptr = fp->_wide_data->_IO_write_ptr; + _IO_off64_t ret = _IO_wstr_seekoff (fp, offset, dir, mode); + mp->seekwriteptr = fp->_wide_data->_IO_write_ptr; + return ret; +}
This patch fixes how open_{w}memstream updates the returned buffer size on a fclose/fflush operation. POSIX states [1]: "After a successful fflush() or fclose() [...] the variable pointed to by sizep shall contain the smaller of the current buffer length and the number of bytes for open_memstream(), or the number of wide characters for open_wmemstream(), between the beginning of the buffer and the current file position indicator." Current GLIBC behavior returns the seek position even if there is no previous write operation. To correctly report the buffer size the implementation must track both the buffer positiob and currently bytes written. However internal _IO_write_ptr is updated on both write and seek operations. This patch fixes it by adding two new internal fields to keep track of both previous and next position after a write operation. It allows the sync and close callbacks to know if a write has occurred based on previous and current _IO_write_ptr value. Tested on x86_64. [BZ #15298] * libio/memstream.c (_IO_FILE_memstream): Add prevwriteptr and seekwriteptr fields. (_IO_mem_seekoff): New function. (_IO_mem_jumps): Use _IO_mem_seekoff. (__open_memstream): Initialize prevwriteptr and seekwriteptr. (_IO_mem_sync): Update sizeloc based on written bytes instead of buffer current position. (_IO_mem_finish): Likewise. * libio/wmemstream.c (_IO_FILE_wmemstream): Add prevwriteptr and seekwriteptr fields. (_IO_wmem_seekoff): New function. (_IO_wmem_jumps): Use _IO_mem_seekoff. (__open_wmemstream): Initialize prevwriteptr and seekwriteptr. (_IO_mem_wsync): Update sizeloc based on written bytes instead of buffer current position. (_IO_mem_wfinish): Likewise. * libio/tst-memstream3.c (do_test_bz15298): New function. (do_test_bz18241): Check for expected size after a fseek followed by a fflush. (do_test): Call do_test_bz15298. [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/open_memstream.html --- ChangeLog | 22 +++++++++++++++ libio/memstream.c | 52 ++++++++++++++++++++++++++++++++---- libio/tst-memstream3.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++--- libio/wmemstream.c | 54 ++++++++++++++++++++++++++++++++----- 4 files changed, 185 insertions(+), 15 deletions(-) -- 2.7.4