public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] libio: Fix seek-past-end returned size for open_{w}memstream (BZ#15298)
@ 2017-04-18 20:44 Adhemerval Zanella
  0 siblings, 0 replies; 2+ messages in thread
From: Adhemerval Zanella @ 2017-04-18 20:44 UTC (permalink / raw)
  To: libc-alpha

This is on my backlog for a long time, so I am resending.  The only change
from the previous iteration [1] is to adapt libio/tst-memstream3.c to use
the support library.  Florian, is my initial response suffice for patch
inclusion?

--

This patch fixes how open_{w}memstream updates the returned buffer
size on a fclose/fflush operation.  POSIX states [2]:

"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] https://www.sourceware.org/ml/libc-alpha/2016-08/msg00837.html
[2] http://pubs.opengroup.org/onlinepubs/9699919799/functions/open_memstream.html
---
 ChangeLog              | 24 ++++++++++++++++
 libio/memstream.c      | 52 ++++++++++++++++++++++++++++++----
 libio/tst-memstream3.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++----
 libio/wmemstream.c     | 54 +++++++++++++++++++++++++++++++-----
 4 files changed, 188 insertions(+), 17 deletions(-)

diff --git a/libio/memstream.c b/libio/memstream.c
index f83d4a5..f600a16 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 ce201d1..f8dc465 100644
--- a/libio/tst-memstream3.c
+++ b/libio/tst-memstream3.c
@@ -59,6 +59,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;
@@ -126,15 +189,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')
@@ -157,11 +222,11 @@ do_test (void)
 
   mcheck_pedantic (mcheck_abort);
 
+  ret += do_test_bz15298 ();
   ret += do_test_bz18241 ();
   ret += do_test_bz20181 ();
 
   return ret;
 }
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/libio/wmemstream.c b/libio/wmemstream.c
index 5bc77f5..24d7e24 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;
+}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH] libio: Fix seek-past-end returned size for open_{w}memstream (BZ#15298)
@ 2017-07-14 18:44 Adhemerval Zanella
  0 siblings, 0 replies; 2+ messages in thread
From: Adhemerval Zanella @ 2017-07-14 18:44 UTC (permalink / raw)
  To: libc-alpha

POSIX states that after a successfull fflush() or fclose() the variable
pointed by the input size shall contain the smaller of the current buffer
length and the number of bytes (or wide characters for wide version) [1].
Current GLIBC behavior returns the seek position even there is no previous
write operation.

To correctly report the buffer size the implementation must track both the
buffer position and current byte written.  However internal _IO_write_ptr
is update on both write and seek operations.

This patch fixes how open_{w}memstream updates the returned buffer size
of a fclose/fflush operation by adding two new internal fields to keep
track of both previous and next position after a write operation.

Checked on x86_64-linux-gnu.

From Florian's comment, this is intended for 2.27.  This patch should
be applied on top on '[PATCH] libio: Flush stream at freopen (BZ#21037)' [2].

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/open_memstream.html
[2] https://sourceware.org/ml/libc-alpha/2017-07/msg00556.html
---
 ChangeLog               | 24 ++++++++++++++++
 libio/Makefile          |  4 +--
 libio/memstream.c       | 61 ++++++++++++++++++++++++++++++++++++----
 libio/tst-memstream3.c  |  8 ++++--
 libio/tst-memstream6.c  | 74 +++++++++++++++++++++++++++++++++++++++++++++++++
 libio/tst-wmemstream6.c | 20 +++++++++++++
 libio/wmemstream.c      | 65 ++++++++++++++++++++++++++++++++++++-------
 7 files changed, 236 insertions(+), 20 deletions(-)
 create mode 100644 libio/tst-memstream6.c
 create mode 100644 libio/tst-wmemstream6.c

diff --git a/libio/Makefile b/libio/Makefile
index 8adf2d8..159ddf1 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -58,9 +58,9 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
 	tst-mmap2-eofsync tst-mmap-offend bug-fopena+ bug-wfflush \
 	bug-ungetc2 bug-ftell bug-ungetc3 bug-ungetc4 tst-fopenloc2 \
 	tst-memstream1 tst-memstream2 tst-memstream3 tst-memstream4 \
-	tst-memstream5 \
+	tst-memstream5 tst-memstream6 \
 	tst-wmemstream1 tst-wmemstream2 tst-wmemstream3 tst-wmemstream4 \
-	tst-wmemstream5 bug-memstream1 bug-wmemstream1 \
+	tst-wmemstream5 tst-wmemstream6 bug-memstream1 bug-wmemstream1 \
 	tst-setvbuf1 tst-popen1 tst-fgetwc bug-wsetpos tst-fseek \
 	tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \
 	tst-ftell-append tst-fputws
diff --git a/libio/memstream.c b/libio/memstream.c
index 67ecc09..7c30400 100644
--- a/libio/memstream.c
+++ b/libio/memstream.c
@@ -26,12 +26,16 @@ 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 int _IO_mem_overflow (_IO_FILE *fp, int c) __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 =
@@ -44,7 +48,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),
@@ -98,11 +102,46 @@ __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 (since some _IO_* function
+     access the pointer directly to optimize updates).  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;
 }
 libc_hidden_def (__open_memstream)
 weak_alias (__open_memstream, open_memstream)
 
+/* Update 'size' with written number of bytes and return true if a written
+   operation has occured.  */
+static bool
+update_bufsize (const _IO_FILE *fp, size_t *size)
+{
+  const struct _IO_FILE_memstream *mp =
+    (const struct _IO_FILE_memstream *) fp;
+  if (fp->_IO_write_ptr == mp->seekwriteptr)
+    {
+      *size = mp->prevwriteptr - fp->_IO_write_base;
+      return false;
+    }
+  *size = fp->_IO_write_ptr - fp->_IO_write_base;
+  return true;
+}
 
 static int
 _IO_mem_sync (_IO_FILE *fp)
@@ -116,7 +155,7 @@ _IO_mem_sync (_IO_FILE *fp)
     }
 
   *mp->bufloc = fp->_IO_write_base;
-  *mp->sizeloc = fp->_IO_write_ptr - fp->_IO_write_base;
+  update_bufsize (fp, mp->sizeloc);
 
   return 0;
 }
@@ -131,8 +170,9 @@ _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;
+      /* An '\0' should be appended iff a write operation ocurred.  */
+      if (update_bufsize (fp, mp->sizeloc))
+	(*mp->bufloc)[*mp->sizeloc] = '\0';
 
       fp->_IO_buf_base = NULL;
     }
@@ -148,8 +188,19 @@ _IO_mem_overflow (_IO_FILE *fp, int c)
       /* Updates the returned size location on stream flush.  */
       struct _IO_FILE_memstream *mp = (struct _IO_FILE_memstream *) fp;
       *mp->bufloc = fp->_IO_write_base;
-      *mp->sizeloc = fp->_IO_write_ptr - fp->_IO_write_base;
+      update_bufsize (fp, mp->sizeloc);
       return 0;
     }
   return _IO_str_overflow (fp, c);
 }
+
+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 ce201d1..6521f92 100644
--- a/libio/tst-memstream3.c
+++ b/libio/tst-memstream3.c
@@ -126,15 +126,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')
diff --git a/libio/tst-memstream6.c b/libio/tst-memstream6.c
new file mode 100644
index 0000000..b877d68
--- /dev/null
+++ b/libio/tst-memstream6.c
@@ -0,0 +1,74 @@
+/* Test for open_memstream BZ #15298.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include "tst-memstream.h"
+
+static void
+mcheck_abort (enum mcheck_status ev)
+{
+  printf ("mecheck failed with status %d\n", (int) ev);
+  exit (1);
+}
+
+static int
+do_test (void)
+{
+  mcheck_pedantic (mcheck_abort);
+
+  {
+    CHAR_T *buf;
+    size_t size;
+
+    FILE *fp = OPEN_MEMSTREAM (&buf, &size);
+    TEST_VERIFY_EXIT (fp != NULL);
+
+    /* Move internal position but do not write any bytes.  Final size should
+       be 0.  */
+    TEST_VERIFY_EXIT (fseek (fp, 10, SEEK_SET) != -1);
+    TEST_VERIFY_EXIT (fseek (fp, 20, SEEK_CUR) != -1);
+    TEST_VERIFY_EXIT (fseek (fp, 30, SEEK_CUR) != -1);
+    TEST_VERIFY_EXIT (fflush (fp) != -1);
+    TEST_VERIFY (size == 0);
+
+    /* Now write some bytes and change internal position.  Final size should
+      be based on written bytes.  */
+    TEST_VERIFY_EXIT (fseek (fp, 0, SEEK_SET) != -1);
+    TEST_VERIFY_EXIT (FWRITE (W("abc"), 1, 3, fp) == 3);
+    TEST_VERIFY_EXIT (fseek (fp, 20, SEEK_CUR) != -1);
+    TEST_VERIFY_EXIT (fseek (fp, 30, SEEK_CUR) != -1);
+    TEST_VERIFY_EXIT (fflush (fp) != -1);
+    TEST_VERIFY (size == 3);
+
+    /* Finally set position, write some bytes and change position again.
+       Final size should be based again on write position.  */
+    size_t offset = 2048;
+    TEST_VERIFY_EXIT (fseek (fp, offset, SEEK_SET) != -1);
+    TEST_VERIFY_EXIT (FWRITE (W("def"), 1, 3, fp) == 3);
+    TEST_VERIFY_EXIT (fseek (fp, 20, SEEK_CUR) != -1);
+    TEST_VERIFY_EXIT (fseek (fp, 20, SEEK_CUR) != -1);
+    TEST_VERIFY_EXIT (fflush (fp) != -1);
+    TEST_VERIFY (size == (offset + 3));
+
+    TEST_VERIFY_EXIT (fclose (fp) == 0);
+    free (buf);
+  }
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/libio/tst-wmemstream6.c b/libio/tst-wmemstream6.c
new file mode 100644
index 0000000..5d8cd75
--- /dev/null
+++ b/libio/tst-wmemstream6.c
@@ -0,0 +1,20 @@
+/* Test for open_wmemstream BZ #15298.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define TEST_WCHAR
+#include <libio/tst-memstream6.c>
diff --git a/libio/wmemstream.c b/libio/wmemstream.c
index d68334b..db54b5e 100644
--- a/libio/wmemstream.c
+++ b/libio/wmemstream.c
@@ -27,12 +27,16 @@ 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 int _IO_wmem_overflow (_IO_FILE *fp, int c) __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 =
@@ -45,7 +49,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),
@@ -99,11 +103,46 @@ 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 (since some _IO_* function
+     access the pointer directly to optimize updates).  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;
 }
 
 
+/* Update 'size' with written number of bytes and return true if a written
+   operation has occured.  */
+static bool
+update_bufsize (const _IO_FILE *fp, size_t *size)
+{
+  const struct _IO_FILE_wmemstream *mp =
+    (const struct _IO_FILE_wmemstream *) fp;
+  if (fp->_wide_data->_IO_write_ptr == mp->seekwriteptr)
+    {
+      *size = mp->prevwriteptr - fp->_wide_data->_IO_write_base;
+      return false;
+    }
+  *size = fp->_wide_data->_IO_write_ptr - fp->_wide_data->_IO_write_base;
+  return true;
+}
+
 static int
 _IO_wmem_sync (_IO_FILE *fp)
 {
@@ -116,13 +155,11 @@ _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);
+  update_bufsize (fp, mp->sizeloc);
 
   return 0;
 }
 
-
 static void
 _IO_wmem_finish (_IO_FILE *fp, int dummy)
 {
@@ -134,10 +171,8 @@ _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';
-      *mp->sizeloc = len;
+      if (update_bufsize (fp, mp->sizeloc))
+	(*mp->bufloc)[*mp->sizeloc] = L'\0';
 
       fp->_wide_data->_IO_buf_base = NULL;
     }
@@ -153,9 +188,19 @@ _IO_wmem_overflow (_IO_FILE *fp, int c)
       /* Updates the returned size location on stream flush.  */
       struct _IO_FILE_wmemstream *mp = (struct _IO_FILE_wmemstream *) fp;
       *mp->bufloc = fp->_wide_data->_IO_write_base;
-      *mp->sizeloc = (fp->_wide_data->_IO_write_ptr
-		     - fp->_wide_data->_IO_write_base);
+      update_bufsize (fp, mp->sizeloc);
       return 0;
     }
   return _IO_wstr_overflow (fp, c);
 }
+
+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;
+}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-07-14 18:44 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-18 20:44 [PATCH] libio: Fix seek-past-end returned size for open_{w}memstream (BZ#15298) Adhemerval Zanella
2017-07-14 18:44 Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).