[v2,1/2] libio: Multiple fixes for open_{w}memstram (BZ#18241 and BZ#20181)

Changes from previous version:

 * Set EINVAL on _IO_{w}str_seekoff and remove INVALPOS
 * Rewrite how to print error messages in tests to avoid put __FILE__ in
   format string.
 * Indentation and style fixes.

--

This patches fixes multiples issues on open_{w}memstream reported on both
BZ#18241 and BZ#20181:

  - failed fseek does not set errno.
  - negative offset in fseek fails even when resulting position is
    a valid one.
  - a flush after write if the current write position is not at the
    end of the stream currupt data.

The main fix is on seek operation for memstream (_IO_{w}str_seekoff), where
both _IO_read_ptr and _IO_read_end pointer are updated if a write operation
has occured (similar to default file operations).  Also, to calculate the
offset on both read and write pointers, a temporary value is instead of
updating the argument supplied value.  Negative offset are valid if resulting
internal pointer is within the range of _IO_{read,write}_base and
_IO_{read,write}_end.

Also POSIX states that a null or wide null shall be appended to the current
buffer iff a write moves the position to a value larger than the current
lenght.  Current implementation appends a null or wide null regardless
of this condition.  This patch fixes it by removing the 'else' condition
on _IO_{w}mem_sync.

Checked on x86_64.

	[BZ #18241]
	[BZ #20181]
	* libio/Makefile (test): Add tst-memstream3 and tst-wmemstream3.
	* libio/memstream.c (_IO_mem_sync): Only append a null byte if
	write position is at the end the buffer.
	* libio/wmemstream.c (_IO_wmem_sync): Likewise.
	* libio/strops.c (_IO_str_switch_to_get_mode): New function.
	(_IO_str_seekoff): Set correct offset from negative displacement and
	set EINVAL for invalid ones.
	* libio/wstrops.c (enlarge_userbuf): Use correct function to calculate
	buffer length.
	(_IO_wstr_switch_to_get_mode): New function.
	(_IO_wstr_seekoff): Set correct offset from negative displacement and
	set EINVAL for invalid ones.
	* libio/tst-memstream3.c: New file.
	* libio/tst-wmemstream3.c: Likewise.
	* manual/examples/memstrm.c: Remove warning when priting size_t.
---
 ChangeLog                 |  20 ++++++
 libio/Makefile            |   4 +-
 libio/memstream.c         |   2 -
 libio/strops.c            |  81 +++++++++++++++--------
 libio/tst-memstream3.c    | 165 ++++++++++++++++++++++++++++++++++++++++++++++
 libio/tst-wmemstream3.c   |  44 +++++++++++++
 libio/wmemstream.c        |   2 -
 libio/wstrops.c           |  89 +++++++++++++++++--------
 manual/examples/memstrm.c |   4 +-
 9 files changed, 348 insertions(+), 63 deletions(-)
 create mode 100644 libio/tst-memstream3.c
 create mode 100644 libio/tst-wmemstream3.c

Ping.

On 08/08/2016 17:43, Adhemerval Zanella wrote:
> Changes from previous version:
> 
>  * Set EINVAL on _IO_{w}str_seekoff and remove INVALPOS
>  * Rewrite how to print error messages in tests to avoid put __FILE__ in
>    format string.
>  * Indentation and style fixes.
> 
> --
> 
> This patches fixes multiples issues on open_{w}memstream reported on both
> BZ#18241 and BZ#20181:
> 
>   - failed fseek does not set errno.
>   - negative offset in fseek fails even when resulting position is
>     a valid one.
>   - a flush after write if the current write position is not at the
>     end of the stream currupt data.
> 
> The main fix is on seek operation for memstream (_IO_{w}str_seekoff), where
> both _IO_read_ptr and _IO_read_end pointer are updated if a write operation
> has occured (similar to default file operations).  Also, to calculate the
> offset on both read and write pointers, a temporary value is instead of
> updating the argument supplied value.  Negative offset are valid if resulting
> internal pointer is within the range of _IO_{read,write}_base and
> _IO_{read,write}_end.
> 
> Also POSIX states that a null or wide null shall be appended to the current
> buffer iff a write moves the position to a value larger than the current
> lenght.  Current implementation appends a null or wide null regardless
> of this condition.  This patch fixes it by removing the 'else' condition
> on _IO_{w}mem_sync.
> 
> Checked on x86_64.
> 
> 	[BZ #18241]
> 	[BZ #20181]
> 	* libio/Makefile (test): Add tst-memstream3 and tst-wmemstream3.
> 	* libio/memstream.c (_IO_mem_sync): Only append a null byte if
> 	write position is at the end the buffer.
> 	* libio/wmemstream.c (_IO_wmem_sync): Likewise.
> 	* libio/strops.c (_IO_str_switch_to_get_mode): New function.
> 	(_IO_str_seekoff): Set correct offset from negative displacement and
> 	set EINVAL for invalid ones.
> 	* libio/wstrops.c (enlarge_userbuf): Use correct function to calculate
> 	buffer length.
> 	(_IO_wstr_switch_to_get_mode): New function.
> 	(_IO_wstr_seekoff): Set correct offset from negative displacement and
> 	set EINVAL for invalid ones.
> 	* libio/tst-memstream3.c: New file.
> 	* libio/tst-wmemstream3.c: Likewise.
> 	* manual/examples/memstrm.c: Remove warning when priting size_t.
> ---
>  ChangeLog                 |  20 ++++++
>  libio/Makefile            |   4 +-
>  libio/memstream.c         |   2 -
>  libio/strops.c            |  81 +++++++++++++++--------
>  libio/tst-memstream3.c    | 165 ++++++++++++++++++++++++++++++++++++++++++++++
>  libio/tst-wmemstream3.c   |  44 +++++++++++++
>  libio/wmemstream.c        |   2 -
>  libio/wstrops.c           |  89 +++++++++++++++++--------
>  manual/examples/memstrm.c |   4 +-
>  9 files changed, 348 insertions(+), 63 deletions(-)
>  create mode 100644 libio/tst-memstream3.c
>  create mode 100644 libio/tst-wmemstream3.c
> 
> diff --git a/ChangeLog b/ChangeLog
> index 205da06..28d9012 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,23 @@
> +2016-08-05  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> +
> +	[BZ #18241]
> +	[BZ #20181]
> +	* libio/Makefile (test): Add tst-memstream3 and tst-wmemstream3.
> +	* libio/memstream.c (_IO_mem_sync): Only append a null byte if
> +	write position is at the end the buffer.
> +	* libio/wmemstream.c (_IO_wmem_sync): Likewise.
> +	* libio/strops.c (_IO_str_switch_to_get_mode): New function.
> +	(_IO_str_seekoff): Set correct offset from negative displacement and
> +	set EINVAL for invalid ones.
> +	* libio/wstrops.c (enlarge_userbuf): Use correct function to calculate
> +	buffer length.
> +	(_IO_wstr_switch_to_get_mode): New function.
> +	(_IO_wstr_seekoff): Set correct offset from negative displacement and
> +	set EINVAL for invalid ones.
> +	* libio/tst-memstream3.c: New file.
> +	* libio/tst-wmemstream3.c: Likewise.
> +	* manual/examples/memstrm.c: Remove warning when priting size_t.
> +
>  2016-08-05  Torvald Riegel  <triegel@redhat.com>
>  
>  	* include/atomic.h (atomic_exchange_relaxed): New.
> diff --git a/libio/Makefile b/libio/Makefile
> index 12589f2..0c7751c 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -56,8 +56,8 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
>  	tst-mmap-eofsync tst-mmap-fflushsync bug-mmap-fflush \
>  	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-wmemstream1 tst-wmemstream2 \
> +	tst-memstream1 tst-memstream2 tst-memstream3 \
> +	tst-wmemstream1 tst-wmemstream2 tst-wmemstream3 \
>  	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 \
> diff --git a/libio/memstream.c b/libio/memstream.c
> index e20b9c2..f1e8d58 100644
> --- a/libio/memstream.c
> +++ b/libio/memstream.c
> @@ -112,8 +112,6 @@ _IO_mem_sync (_IO_FILE *fp)
>        _IO_str_overflow (fp, '\0');
>        --fp->_IO_write_ptr;
>      }
> -  else
> -    *fp->_IO_write_ptr = '\0';
>  
>    *mp->bufloc = fp->_IO_write_base;
>    *mp->sizeloc = fp->_IO_write_ptr - fp->_IO_write_base;
> diff --git a/libio/strops.c b/libio/strops.c
> index 2ba3704..1bb8a77 100644
> --- a/libio/strops.c
> +++ b/libio/strops.c
> @@ -230,6 +230,21 @@ enlarge_userbuf (_IO_FILE *fp, _IO_off64_t offset, int reading)
>    return 0;
>  }
>  
> +static void
> +_IO_str_switch_to_get_mode (_IO_FILE *fp)
> +{
> +  if (_IO_in_backup (fp))
> +    fp->_IO_read_base = fp->_IO_backup_base;
> +  else
> +    {
> +      fp->_IO_read_base = fp->_IO_buf_base;
> +      if (fp->_IO_write_ptr > fp->_IO_read_end)
> +        fp->_IO_read_end = fp->_IO_write_ptr;
> +    }
> +  fp->_IO_read_ptr = fp->_IO_read_end = fp->_IO_write_ptr;
> +
> +  fp->_flags &= ~_IO_CURRENTLY_PUTTING;
> +}
>  
>  _IO_off64_t
>  _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
> @@ -239,14 +254,14 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>    if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET))
>      mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT);
>  
> +  bool was_writing = (fp->_IO_write_ptr > fp->_IO_write_base
> +		     || _IO_in_put_mode (fp));
> +  if (was_writing)
> +    _IO_str_switch_to_get_mode (fp);
> +
>    if (mode == 0)
>      {
> -      /* Don't move any pointers. But there is no clear indication what
> -	 mode FP is in. Let's guess. */
> -      if (fp->_IO_file_flags & _IO_NO_WRITES)
> -        new_pos = fp->_IO_read_ptr - fp->_IO_read_base;
> -      else
> -        new_pos = fp->_IO_write_ptr - fp->_IO_write_base;
> +      new_pos = fp->_IO_read_ptr - fp->_IO_read_base;
>      }
>    else
>      {
> @@ -256,48 +271,62 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>        /* Move the get pointer, if requested. */
>        if (mode & _IOS_INPUT)
>  	{
> +	  _IO_ssize_t base;
>  	  switch (dir)
>  	    {
> -	    case _IO_seek_end:
> -	      offset += cur_size;
> +	    case _IO_seek_set:
> +	      base = 0;
>  	      break;
>  	    case _IO_seek_cur:
> -	      offset += fp->_IO_read_ptr - fp->_IO_read_base;
> +	      base = fp->_IO_read_ptr - fp->_IO_read_base;
>  	      break;
> -	    default: /* case _IO_seek_set: */
> +	    default: /* case _IO_seek_end: */
> +	      base = cur_size;
>  	      break;
>  	    }
> -	  if (offset < 0)
> -	    return EOF;
> -	  if ((_IO_ssize_t) offset > cur_size
> -	      && enlarge_userbuf (fp, offset, 1) != 0)
> +	  _IO_ssize_t maxval = SSIZE_MAX - base;
> +	  if (offset < -base || offset > maxval)
> +	    {
> +	      __set_errno (EINVAL);
> +	      return EOF;
> +	    }
> +	  base += offset;
> +	  if (base > cur_size
> +	      && enlarge_userbuf (fp, base, 1) != 0)
>  	    return EOF;
> -	  fp->_IO_read_ptr = fp->_IO_read_base + offset;
> +	  fp->_IO_read_ptr = fp->_IO_read_base + base;
>  	  fp->_IO_read_end = fp->_IO_read_base + cur_size;
> -	  new_pos = offset;
> +	  new_pos = base;
>  	}
>  
>        /* Move the put pointer, if requested. */
>        if (mode & _IOS_OUTPUT)
>  	{
> +	  _IO_ssize_t base;
>  	  switch (dir)
>  	    {
> -	    case _IO_seek_end:
> -	      offset += cur_size;
> +	    case _IO_seek_set:
> +	      base = 0;
>  	      break;
>  	    case _IO_seek_cur:
> -	      offset += fp->_IO_write_ptr - fp->_IO_write_base;
> +	      base = fp->_IO_write_ptr - fp->_IO_write_base;
>  	      break;
> -	    default: /* case _IO_seek_set: */
> +	    default: /* case _IO_seek_end: */
> +	      base = cur_size;
>  	      break;
>  	    }
> -	  if (offset < 0)
> -	    return EOF;
> -	  if ((_IO_ssize_t) offset > cur_size
> -	      && enlarge_userbuf (fp, offset, 0) != 0)
> +	  _IO_ssize_t maxval = SSIZE_MAX - base;
> +	  if (offset < -base || offset > maxval)
> +	    {
> +	      __set_errno (EINVAL);
> +	      return EOF;
> +	    }
> +	  base += offset;
> +	  if (base > cur_size
> +	      && enlarge_userbuf (fp, base, 0) != 0)
>  	    return EOF;
> -	  fp->_IO_write_ptr = fp->_IO_write_base + offset;
> -	  new_pos = offset;
> +	  fp->_IO_write_ptr = fp->_IO_write_base + base;
> +	  new_pos = base;
>  	}
>      }
>    return new_pos;
> diff --git a/libio/tst-memstream3.c b/libio/tst-memstream3.c
> new file mode 100644
> index 0000000..34b04e5
> --- /dev/null
> +++ b/libio/tst-memstream3.c
> @@ -0,0 +1,165 @@
> +/* Test for open_memstream implementation.
> +   Copyright (C) 2016 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 <mcheck.h>
> +#include <stdio.h>
> +#include <stdarg.h>
> +#include <errno.h>
> +
> +
> +#ifndef CHAR_T
> +# define CHAR_T char
> +# define W(o) o
> +# define OPEN_MEMSTREAM open_memstream
> +# define PRINTF printf
> +# define FWRITE fwrite
> +# define FPUTC fputc
> +# define STRCMP strcmp
> +#endif
> +
> +#define S(s) S1 (s)
> +#define S1(s) #s
> +
> +static void
> +mcheck_abort (enum mcheck_status ev)
> +{
> +  printf ("mecheck failed with status %d\n", (int) ev);
> +  exit (1);
> +}
> +
> +static void
> +error_printf (int line, const char *fmt, ...)
> +{
> +  va_list ap;
> +
> +  printf ("error: %s:%i: ", __FILE__, line);
> +  va_start (ap, fmt);
> +  vprintf (fmt, ap);
> +  va_end (ap);
> +}
> +
> +#define ERROR_RET1(...) \
> +  { error_printf(__LINE__, __VA_ARGS__); return 1; }
> +
> +static int
> +do_test_bz18241 (void)
> +{
> +  CHAR_T *buf;
> +  size_t size;
> +
> +  FILE *fp = OPEN_MEMSTREAM (&buf, &size);
> +  if (fp == NULL)
> +    ERROR_RET1 ("%s failed\n", S(OPEN_MEMSTREAM));
> +
> +  if (FPUTC (W('a'), fp) != W('a'))
> +    ERROR_RET1 ("%s failed (errno = %d)\n", S(FPUTC), errno);
> +  if (fflush (fp) != 0)
> +    ERROR_RET1 ("fflush failed (errno = %d)\n", errno);
> +  if (fseek (fp, -2, SEEK_SET) != -1)
> +    ERROR_RET1 ("fseek failed (errno = %d)\n", errno);
> +  if (errno != EINVAL)
> +    ERROR_RET1 ("errno != EINVAL\n");
> +  if (ftell (fp) != 1)
> +    ERROR_RET1 ("ftell failed (errno = %d)\n", errno);
> +  if (ferror (fp) != 0)
> +    ERROR_RET1 ("ferror != 0\n");
> +
> +  if (fseek (fp, -1, SEEK_CUR) == -1)
> +    ERROR_RET1 ("fseek failed (errno = %d)\n", errno);
> +  if (ftell (fp) != 0)
> +    ERROR_RET1 ("ftell failed (errno = %d)\n", errno);
> +  if (ferror (fp) != 0)
> +    ERROR_RET1 ("ferror != 0\n");
> +  if (FPUTC (W('b'), fp) != W('b'))
> +    ERROR_RET1 ("%s failed (errno = %d)\n", S(FPUTC), errno);
> +  if (fflush (fp) != 0)
> +    ERROR_RET1 ("fflush failed (errno = %d)\n", errno);
> +
> +  if (fclose (fp) != 0)
> +    ERROR_RET1 ("fclose failed (errno = %d\n", errno);
> +
> +  if (STRCMP (buf, W("b")) != 0)
> +    ERROR_RET1 ("%s failed\n", S(STRCMP));
> +
> +  free (buf);
> +
> +  return 0;
> +}
> +
> +static int
> +do_test_bz20181 (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));
> +
> +  if ((ret = FWRITE (W("abc"), 1, 3, fp)) != 3)
> +    ERROR_RET1 ("%s failed (errno = %d)\n", S(FWRITE), errno);
> +
> +  if (fseek (fp, 0, SEEK_SET) != 0)
> +    ERROR_RET1 ("fseek failed (errno = %d)\n", errno);
> +
> +  if (FWRITE (W("z"), 1, 1, fp) != 1)
> +    ERROR_RET1 ("%s failed (errno = %d)\n", S(FWRITE), errno);
> +
> +  if (fflush (fp) != 0)
> +    ERROR_RET1 ("fflush failed (errno = %d)\n", errno);
> +
> +  /* Avoid truncating the buffer on close.  */
> +  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 (buf[0] != W('z')
> +      || buf[1] != W('b')
> +      || buf[2] != W('c'))
> +    {
> +      PRINTF (W("error: buf {%c,%c,%c} != {z,b,c}\n"),
> +	      buf[0], buf[1], buf[2]);
> +      return 1;
> +    }
> +    
> +  free (buf);
> +
> +  return 0;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  int ret = 0;
> +
> +  mcheck_pedantic (mcheck_abort);
> +
> +  ret += do_test_bz18241 ();
> +  ret += do_test_bz20181 ();
> +
> +  return ret;
> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"
> diff --git a/libio/tst-wmemstream3.c b/libio/tst-wmemstream3.c
> new file mode 100644
> index 0000000..190283a
> --- /dev/null
> +++ b/libio/tst-wmemstream3.c
> @@ -0,0 +1,44 @@
> +/* Test for open_memstream implementation.
> +   Copyright (C) 2016 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 <wchar.h>
> +
> +/* Straighforward implementation so tst-memstream3 could use check
> +   fwrite on open_memstream.  */
> +static size_t
> +fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq)
> +{
> +  const wchar_t *wcs = (const wchar_t*) (ptr);
> +  for (size_t s = 0; s < size; s++)
> +    {
> +      for (size_t n = 0; n < nmemb; n++)
> +        if (fputwc (wcs[n], arq) == WEOF)
> +          return n; 
> +    }
> +  return size * nmemb; 
> +}
> +
> +#define CHAR_T wchar_t
> +#define W(o) L##o
> +#define OPEN_MEMSTREAM open_wmemstream
> +#define PRINTF wprintf
> +#define FWRITE fwwrite
> +#define FPUTC  fputwc
> +#define STRCMP wcscmp
> +
> +#include "tst-memstream3.c"
> diff --git a/libio/wmemstream.c b/libio/wmemstream.c
> index bf2a50b..fd01be0 100644
> --- a/libio/wmemstream.c
> +++ b/libio/wmemstream.c
> @@ -112,8 +112,6 @@ _IO_wmem_sync (_IO_FILE *fp)
>        _IO_wstr_overflow (fp, '\0');
>        --fp->_wide_data->_IO_write_ptr;
>      }
> -  else
> -    *fp->_wide_data->_IO_write_ptr = '\0';
>  
>    *mp->bufloc = fp->_wide_data->_IO_write_base;
>    *mp->sizeloc = (fp->_wide_data->_IO_write_ptr
> diff --git a/libio/wstrops.c b/libio/wstrops.c
> index 09fa543..0b2bec3 100644
> --- a/libio/wstrops.c
> +++ b/libio/wstrops.c
> @@ -169,7 +169,7 @@ _IO_wstr_count (_IO_FILE *fp)
>  static int
>  enlarge_userbuf (_IO_FILE *fp, _IO_off64_t offset, int reading)
>  {
> -  if ((_IO_ssize_t) offset <= _IO_blen (fp))
> +  if ((_IO_ssize_t) offset <= _IO_wblen (fp))
>      return 0;
>  
>    struct _IO_wide_data *wd = fp->_wide_data;
> @@ -235,6 +235,22 @@ enlarge_userbuf (_IO_FILE *fp, _IO_off64_t offset, int reading)
>    return 0;
>  }
>  
> +static void
> +_IO_wstr_switch_to_get_mode (_IO_FILE *fp)
> +{
> +  if (_IO_in_backup (fp))
> +    fp->_wide_data->_IO_read_base = fp->_wide_data->_IO_backup_base;
> +  else
> +    {
> +      fp->_wide_data->_IO_read_base = fp->_wide_data->_IO_buf_base;
> +      if (fp->_wide_data->_IO_write_ptr > fp->_wide_data->_IO_read_end)
> +        fp->_wide_data->_IO_read_end = fp->_wide_data->_IO_write_ptr;
> +    }
> +  fp->_wide_data->_IO_read_ptr = fp->_wide_data->_IO_write_ptr;
> +  fp->_wide_data->_IO_read_end = fp->_wide_data->_IO_write_ptr;
> +
> +  fp->_flags &= ~_IO_CURRENTLY_PUTTING;
> +}
>  
>  _IO_off64_t
>  _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
> @@ -244,15 +260,16 @@ _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>    if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET))
>      mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT);
>  
> +  bool was_writing = (fp->_wide_data->_IO_write_ptr >
> +			fp->_wide_data->_IO_write_base
> +		     || _IO_in_put_mode (fp));
> +  if (was_writing)
> +    _IO_wstr_switch_to_get_mode (fp);
> +
>    if (mode == 0)
>      {
> -      /* Don't move any pointers. But there is no clear indication what
> -	 mode FP is in. Let's guess. */
> -      if (fp->_IO_file_flags & _IO_NO_WRITES)
> -        new_pos = fp->_wide_data->_IO_read_ptr - fp->_wide_data->_IO_read_base;
> -      else
> -        new_pos = (fp->_wide_data->_IO_write_ptr
> -		   - fp->_wide_data->_IO_write_base);
> +      new_pos = (fp->_wide_data->_IO_write_ptr
> +		 - fp->_wide_data->_IO_write_base);
>      }
>    else
>      {
> @@ -262,25 +279,32 @@ _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>        /* Move the get pointer, if requested. */
>        if (mode & _IOS_INPUT)
>  	{
> +	  _IO_ssize_t base;
>  	  switch (dir)
>  	    {
> -	    case _IO_seek_end:
> -	      offset += cur_size;
> +	    case _IO_seek_set:
> +	      base = 0;
>  	      break;
>  	    case _IO_seek_cur:
> -	      offset += (fp->_wide_data->_IO_read_ptr
> -			 - fp->_wide_data->_IO_read_base);
> +	      base = (fp->_wide_data->_IO_read_ptr
> +		     - fp->_wide_data->_IO_read_base);
>  	      break;
> -	    default: /* case _IO_seek_set: */
> +	    default: /* case _IO_seek_end: */
> +	      base = cur_size;
>  	      break;
>  	    }
> -	  if (offset < 0)
> -	    return EOF;
> -	  if ((_IO_ssize_t) offset > cur_size
> -	      && enlarge_userbuf (fp, offset, 1) != 0)
> +	  _IO_ssize_t maxval = SSIZE_MAX/sizeof (wchar_t) - base;
> +	  if (offset < -base || offset > maxval)
> +	    {
> +	      __set_errno (EINVAL);
> +	      return EOF;
> +	    }
> +	  base += offset;
> +	  if (base > cur_size
> +	      && enlarge_userbuf (fp, base, 1) != 0)
>  	    return EOF;
>  	  fp->_wide_data->_IO_read_ptr = (fp->_wide_data->_IO_read_base
> -					  + offset);
> +					  + base);
>  	  fp->_wide_data->_IO_read_end = (fp->_wide_data->_IO_read_base
>  					  + cur_size);
>  	  new_pos = offset;
> @@ -289,26 +313,33 @@ _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>        /* Move the put pointer, if requested. */
>        if (mode & _IOS_OUTPUT)
>  	{
> +	  _IO_ssize_t base;
>  	  switch (dir)
>  	    {
> -	    case _IO_seek_end:
> -	      offset += cur_size;
> +	    case _IO_seek_set:
> +	      base = 0;
>  	      break;
>  	    case _IO_seek_cur:
> -	      offset += (fp->_wide_data->_IO_write_ptr
> -			 - fp->_wide_data->_IO_write_base);
> +	      base = (fp->_wide_data->_IO_write_ptr
> +		     - fp->_wide_data->_IO_write_base);
>  	      break;
> -	    default: /* case _IO_seek_set: */
> +	    default: /* case _IO_seek_end: */
> +	      base = cur_size;
>  	      break;
>  	    }
> -	  if (offset < 0)
> -	    return EOF;
> -	  if ((_IO_ssize_t) offset > cur_size
> -	      && enlarge_userbuf (fp, offset, 0) != 0)
> +	  _IO_ssize_t maxval = SSIZE_MAX/sizeof (wchar_t) - base;
> +	  if (offset < -base || offset > maxval)
> +	    {
> +	      __set_errno (EINVAL);
> +	      return EOF;
> +	    }
> +	  base += offset;
> +	  if (base > cur_size
> +	      && enlarge_userbuf (fp, base, 0) != 0)
>  	    return EOF;
>  	  fp->_wide_data->_IO_write_ptr = (fp->_wide_data->_IO_write_base
> -					   + offset);
> -	  new_pos = offset;
> +					   + base);
> +	  new_pos = base;
>  	}
>      }
>    return new_pos;
> diff --git a/manual/examples/memstrm.c b/manual/examples/memstrm.c
> index 0d443b1..5701ba1 100644
> --- a/manual/examples/memstrm.c
> +++ b/manual/examples/memstrm.c
> @@ -27,10 +27,10 @@ main (void)
>    stream = open_memstream (&bp, &size);
>    fprintf (stream, "hello");
>    fflush (stream);
> -  printf ("buf = `%s', size = %d\n", bp, size);
> +  printf ("buf = `%s', size = %zu\n", bp, size);
>    fprintf (stream, ", world");
>    fclose (stream);
> -  printf ("buf = `%s', size = %d\n", bp, size);
> +  printf ("buf = `%s', size = %zu\n", bp, size);
>  
>    return 0;
>  }
>

Ping.

On 25/08/2016 12:53, Adhemerval Zanella wrote:
> Ping.
> 
> On 08/08/2016 17:43, Adhemerval Zanella wrote:
>> Changes from previous version:
>>
>>  * Set EINVAL on _IO_{w}str_seekoff and remove INVALPOS
>>  * Rewrite how to print error messages in tests to avoid put __FILE__ in
>>    format string.
>>  * Indentation and style fixes.
>>
>> --
>>
>> This patches fixes multiples issues on open_{w}memstream reported on both
>> BZ#18241 and BZ#20181:
>>
>>   - failed fseek does not set errno.
>>   - negative offset in fseek fails even when resulting position is
>>     a valid one.
>>   - a flush after write if the current write position is not at the
>>     end of the stream currupt data.
>>
>> The main fix is on seek operation for memstream (_IO_{w}str_seekoff), where
>> both _IO_read_ptr and _IO_read_end pointer are updated if a write operation
>> has occured (similar to default file operations).  Also, to calculate the
>> offset on both read and write pointers, a temporary value is instead of
>> updating the argument supplied value.  Negative offset are valid if resulting
>> internal pointer is within the range of _IO_{read,write}_base and
>> _IO_{read,write}_end.
>>
>> Also POSIX states that a null or wide null shall be appended to the current
>> buffer iff a write moves the position to a value larger than the current
>> lenght.  Current implementation appends a null or wide null regardless
>> of this condition.  This patch fixes it by removing the 'else' condition
>> on _IO_{w}mem_sync.
>>
>> Checked on x86_64.
>>
>> 	[BZ #18241]
>> 	[BZ #20181]
>> 	* libio/Makefile (test): Add tst-memstream3 and tst-wmemstream3.
>> 	* libio/memstream.c (_IO_mem_sync): Only append a null byte if
>> 	write position is at the end the buffer.
>> 	* libio/wmemstream.c (_IO_wmem_sync): Likewise.
>> 	* libio/strops.c (_IO_str_switch_to_get_mode): New function.
>> 	(_IO_str_seekoff): Set correct offset from negative displacement and
>> 	set EINVAL for invalid ones.
>> 	* libio/wstrops.c (enlarge_userbuf): Use correct function to calculate
>> 	buffer length.
>> 	(_IO_wstr_switch_to_get_mode): New function.
>> 	(_IO_wstr_seekoff): Set correct offset from negative displacement and
>> 	set EINVAL for invalid ones.
>> 	* libio/tst-memstream3.c: New file.
>> 	* libio/tst-wmemstream3.c: Likewise.
>> 	* manual/examples/memstrm.c: Remove warning when priting size_t.
>> ---
>>  ChangeLog                 |  20 ++++++
>>  libio/Makefile            |   4 +-
>>  libio/memstream.c         |   2 -
>>  libio/strops.c            |  81 +++++++++++++++--------
>>  libio/tst-memstream3.c    | 165 ++++++++++++++++++++++++++++++++++++++++++++++
>>  libio/tst-wmemstream3.c   |  44 +++++++++++++
>>  libio/wmemstream.c        |   2 -
>>  libio/wstrops.c           |  89 +++++++++++++++++--------
>>  manual/examples/memstrm.c |   4 +-
>>  9 files changed, 348 insertions(+), 63 deletions(-)
>>  create mode 100644 libio/tst-memstream3.c
>>  create mode 100644 libio/tst-wmemstream3.c
>>
>> diff --git a/ChangeLog b/ChangeLog
>> index 205da06..28d9012 100644
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,3 +1,23 @@
>> +2016-08-05  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>> +
>> +	[BZ #18241]
>> +	[BZ #20181]
>> +	* libio/Makefile (test): Add tst-memstream3 and tst-wmemstream3.
>> +	* libio/memstream.c (_IO_mem_sync): Only append a null byte if
>> +	write position is at the end the buffer.
>> +	* libio/wmemstream.c (_IO_wmem_sync): Likewise.
>> +	* libio/strops.c (_IO_str_switch_to_get_mode): New function.
>> +	(_IO_str_seekoff): Set correct offset from negative displacement and
>> +	set EINVAL for invalid ones.
>> +	* libio/wstrops.c (enlarge_userbuf): Use correct function to calculate
>> +	buffer length.
>> +	(_IO_wstr_switch_to_get_mode): New function.
>> +	(_IO_wstr_seekoff): Set correct offset from negative displacement and
>> +	set EINVAL for invalid ones.
>> +	* libio/tst-memstream3.c: New file.
>> +	* libio/tst-wmemstream3.c: Likewise.
>> +	* manual/examples/memstrm.c: Remove warning when priting size_t.
>> +
>>  2016-08-05  Torvald Riegel  <triegel@redhat.com>
>>  
>>  	* include/atomic.h (atomic_exchange_relaxed): New.
>> diff --git a/libio/Makefile b/libio/Makefile
>> index 12589f2..0c7751c 100644
>> --- a/libio/Makefile
>> +++ b/libio/Makefile
>> @@ -56,8 +56,8 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
>>  	tst-mmap-eofsync tst-mmap-fflushsync bug-mmap-fflush \
>>  	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-wmemstream1 tst-wmemstream2 \
>> +	tst-memstream1 tst-memstream2 tst-memstream3 \
>> +	tst-wmemstream1 tst-wmemstream2 tst-wmemstream3 \
>>  	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 \
>> diff --git a/libio/memstream.c b/libio/memstream.c
>> index e20b9c2..f1e8d58 100644
>> --- a/libio/memstream.c
>> +++ b/libio/memstream.c
>> @@ -112,8 +112,6 @@ _IO_mem_sync (_IO_FILE *fp)
>>        _IO_str_overflow (fp, '\0');
>>        --fp->_IO_write_ptr;
>>      }
>> -  else
>> -    *fp->_IO_write_ptr = '\0';
>>  
>>    *mp->bufloc = fp->_IO_write_base;
>>    *mp->sizeloc = fp->_IO_write_ptr - fp->_IO_write_base;
>> diff --git a/libio/strops.c b/libio/strops.c
>> index 2ba3704..1bb8a77 100644
>> --- a/libio/strops.c
>> +++ b/libio/strops.c
>> @@ -230,6 +230,21 @@ enlarge_userbuf (_IO_FILE *fp, _IO_off64_t offset, int reading)
>>    return 0;
>>  }
>>  
>> +static void
>> +_IO_str_switch_to_get_mode (_IO_FILE *fp)
>> +{
>> +  if (_IO_in_backup (fp))
>> +    fp->_IO_read_base = fp->_IO_backup_base;
>> +  else
>> +    {
>> +      fp->_IO_read_base = fp->_IO_buf_base;
>> +      if (fp->_IO_write_ptr > fp->_IO_read_end)
>> +        fp->_IO_read_end = fp->_IO_write_ptr;
>> +    }
>> +  fp->_IO_read_ptr = fp->_IO_read_end = fp->_IO_write_ptr;
>> +
>> +  fp->_flags &= ~_IO_CURRENTLY_PUTTING;
>> +}
>>  
>>  _IO_off64_t
>>  _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>> @@ -239,14 +254,14 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>>    if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET))
>>      mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT);
>>  
>> +  bool was_writing = (fp->_IO_write_ptr > fp->_IO_write_base
>> +		     || _IO_in_put_mode (fp));
>> +  if (was_writing)
>> +    _IO_str_switch_to_get_mode (fp);
>> +
>>    if (mode == 0)
>>      {
>> -      /* Don't move any pointers. But there is no clear indication what
>> -	 mode FP is in. Let's guess. */
>> -      if (fp->_IO_file_flags & _IO_NO_WRITES)
>> -        new_pos = fp->_IO_read_ptr - fp->_IO_read_base;
>> -      else
>> -        new_pos = fp->_IO_write_ptr - fp->_IO_write_base;
>> +      new_pos = fp->_IO_read_ptr - fp->_IO_read_base;
>>      }
>>    else
>>      {
>> @@ -256,48 +271,62 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>>        /* Move the get pointer, if requested. */
>>        if (mode & _IOS_INPUT)
>>  	{
>> +	  _IO_ssize_t base;
>>  	  switch (dir)
>>  	    {
>> -	    case _IO_seek_end:
>> -	      offset += cur_size;
>> +	    case _IO_seek_set:
>> +	      base = 0;
>>  	      break;
>>  	    case _IO_seek_cur:
>> -	      offset += fp->_IO_read_ptr - fp->_IO_read_base;
>> +	      base = fp->_IO_read_ptr - fp->_IO_read_base;
>>  	      break;
>> -	    default: /* case _IO_seek_set: */
>> +	    default: /* case _IO_seek_end: */
>> +	      base = cur_size;
>>  	      break;
>>  	    }
>> -	  if (offset < 0)
>> -	    return EOF;
>> -	  if ((_IO_ssize_t) offset > cur_size
>> -	      && enlarge_userbuf (fp, offset, 1) != 0)
>> +	  _IO_ssize_t maxval = SSIZE_MAX - base;
>> +	  if (offset < -base || offset > maxval)
>> +	    {
>> +	      __set_errno (EINVAL);
>> +	      return EOF;
>> +	    }
>> +	  base += offset;
>> +	  if (base > cur_size
>> +	      && enlarge_userbuf (fp, base, 1) != 0)
>>  	    return EOF;
>> -	  fp->_IO_read_ptr = fp->_IO_read_base + offset;
>> +	  fp->_IO_read_ptr = fp->_IO_read_base + base;
>>  	  fp->_IO_read_end = fp->_IO_read_base + cur_size;
>> -	  new_pos = offset;
>> +	  new_pos = base;
>>  	}
>>  
>>        /* Move the put pointer, if requested. */
>>        if (mode & _IOS_OUTPUT)
>>  	{
>> +	  _IO_ssize_t base;
>>  	  switch (dir)
>>  	    {
>> -	    case _IO_seek_end:
>> -	      offset += cur_size;
>> +	    case _IO_seek_set:
>> +	      base = 0;
>>  	      break;
>>  	    case _IO_seek_cur:
>> -	      offset += fp->_IO_write_ptr - fp->_IO_write_base;
>> +	      base = fp->_IO_write_ptr - fp->_IO_write_base;
>>  	      break;
>> -	    default: /* case _IO_seek_set: */
>> +	    default: /* case _IO_seek_end: */
>> +	      base = cur_size;
>>  	      break;
>>  	    }
>> -	  if (offset < 0)
>> -	    return EOF;
>> -	  if ((_IO_ssize_t) offset > cur_size
>> -	      && enlarge_userbuf (fp, offset, 0) != 0)
>> +	  _IO_ssize_t maxval = SSIZE_MAX - base;
>> +	  if (offset < -base || offset > maxval)
>> +	    {
>> +	      __set_errno (EINVAL);
>> +	      return EOF;
>> +	    }
>> +	  base += offset;
>> +	  if (base > cur_size
>> +	      && enlarge_userbuf (fp, base, 0) != 0)
>>  	    return EOF;
>> -	  fp->_IO_write_ptr = fp->_IO_write_base + offset;
>> -	  new_pos = offset;
>> +	  fp->_IO_write_ptr = fp->_IO_write_base + base;
>> +	  new_pos = base;
>>  	}
>>      }
>>    return new_pos;
>> diff --git a/libio/tst-memstream3.c b/libio/tst-memstream3.c
>> new file mode 100644
>> index 0000000..34b04e5
>> --- /dev/null
>> +++ b/libio/tst-memstream3.c
>> @@ -0,0 +1,165 @@
>> +/* Test for open_memstream implementation.
>> +   Copyright (C) 2016 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 <mcheck.h>
>> +#include <stdio.h>
>> +#include <stdarg.h>
>> +#include <errno.h>
>> +
>> +
>> +#ifndef CHAR_T
>> +# define CHAR_T char
>> +# define W(o) o
>> +# define OPEN_MEMSTREAM open_memstream
>> +# define PRINTF printf
>> +# define FWRITE fwrite
>> +# define FPUTC fputc
>> +# define STRCMP strcmp
>> +#endif
>> +
>> +#define S(s) S1 (s)
>> +#define S1(s) #s
>> +
>> +static void
>> +mcheck_abort (enum mcheck_status ev)
>> +{
>> +  printf ("mecheck failed with status %d\n", (int) ev);
>> +  exit (1);
>> +}
>> +
>> +static void
>> +error_printf (int line, const char *fmt, ...)
>> +{
>> +  va_list ap;
>> +
>> +  printf ("error: %s:%i: ", __FILE__, line);
>> +  va_start (ap, fmt);
>> +  vprintf (fmt, ap);
>> +  va_end (ap);
>> +}
>> +
>> +#define ERROR_RET1(...) \
>> +  { error_printf(__LINE__, __VA_ARGS__); return 1; }
>> +
>> +static int
>> +do_test_bz18241 (void)
>> +{
>> +  CHAR_T *buf;
>> +  size_t size;
>> +
>> +  FILE *fp = OPEN_MEMSTREAM (&buf, &size);
>> +  if (fp == NULL)
>> +    ERROR_RET1 ("%s failed\n", S(OPEN_MEMSTREAM));
>> +
>> +  if (FPUTC (W('a'), fp) != W('a'))
>> +    ERROR_RET1 ("%s failed (errno = %d)\n", S(FPUTC), errno);
>> +  if (fflush (fp) != 0)
>> +    ERROR_RET1 ("fflush failed (errno = %d)\n", errno);
>> +  if (fseek (fp, -2, SEEK_SET) != -1)
>> +    ERROR_RET1 ("fseek failed (errno = %d)\n", errno);
>> +  if (errno != EINVAL)
>> +    ERROR_RET1 ("errno != EINVAL\n");
>> +  if (ftell (fp) != 1)
>> +    ERROR_RET1 ("ftell failed (errno = %d)\n", errno);
>> +  if (ferror (fp) != 0)
>> +    ERROR_RET1 ("ferror != 0\n");
>> +
>> +  if (fseek (fp, -1, SEEK_CUR) == -1)
>> +    ERROR_RET1 ("fseek failed (errno = %d)\n", errno);
>> +  if (ftell (fp) != 0)
>> +    ERROR_RET1 ("ftell failed (errno = %d)\n", errno);
>> +  if (ferror (fp) != 0)
>> +    ERROR_RET1 ("ferror != 0\n");
>> +  if (FPUTC (W('b'), fp) != W('b'))
>> +    ERROR_RET1 ("%s failed (errno = %d)\n", S(FPUTC), errno);
>> +  if (fflush (fp) != 0)
>> +    ERROR_RET1 ("fflush failed (errno = %d)\n", errno);
>> +
>> +  if (fclose (fp) != 0)
>> +    ERROR_RET1 ("fclose failed (errno = %d\n", errno);
>> +
>> +  if (STRCMP (buf, W("b")) != 0)
>> +    ERROR_RET1 ("%s failed\n", S(STRCMP));
>> +
>> +  free (buf);
>> +
>> +  return 0;
>> +}
>> +
>> +static int
>> +do_test_bz20181 (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));
>> +
>> +  if ((ret = FWRITE (W("abc"), 1, 3, fp)) != 3)
>> +    ERROR_RET1 ("%s failed (errno = %d)\n", S(FWRITE), errno);
>> +
>> +  if (fseek (fp, 0, SEEK_SET) != 0)
>> +    ERROR_RET1 ("fseek failed (errno = %d)\n", errno);
>> +
>> +  if (FWRITE (W("z"), 1, 1, fp) != 1)
>> +    ERROR_RET1 ("%s failed (errno = %d)\n", S(FWRITE), errno);
>> +
>> +  if (fflush (fp) != 0)
>> +    ERROR_RET1 ("fflush failed (errno = %d)\n", errno);
>> +
>> +  /* Avoid truncating the buffer on close.  */
>> +  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 (buf[0] != W('z')
>> +      || buf[1] != W('b')
>> +      || buf[2] != W('c'))
>> +    {
>> +      PRINTF (W("error: buf {%c,%c,%c} != {z,b,c}\n"),
>> +	      buf[0], buf[1], buf[2]);
>> +      return 1;
>> +    }
>> +    
>> +  free (buf);
>> +
>> +  return 0;
>> +}
>> +
>> +static int
>> +do_test (void)
>> +{
>> +  int ret = 0;
>> +
>> +  mcheck_pedantic (mcheck_abort);
>> +
>> +  ret += do_test_bz18241 ();
>> +  ret += do_test_bz20181 ();
>> +
>> +  return ret;
>> +}
>> +
>> +#define TEST_FUNCTION do_test ()
>> +#include "../test-skeleton.c"
>> diff --git a/libio/tst-wmemstream3.c b/libio/tst-wmemstream3.c
>> new file mode 100644
>> index 0000000..190283a
>> --- /dev/null
>> +++ b/libio/tst-wmemstream3.c
>> @@ -0,0 +1,44 @@
>> +/* Test for open_memstream implementation.
>> +   Copyright (C) 2016 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 <wchar.h>
>> +
>> +/* Straighforward implementation so tst-memstream3 could use check
>> +   fwrite on open_memstream.  */
>> +static size_t
>> +fwwrite (const void *ptr, size_t size, size_t nmemb, FILE *arq)
>> +{
>> +  const wchar_t *wcs = (const wchar_t*) (ptr);
>> +  for (size_t s = 0; s < size; s++)
>> +    {
>> +      for (size_t n = 0; n < nmemb; n++)
>> +        if (fputwc (wcs[n], arq) == WEOF)
>> +          return n; 
>> +    }
>> +  return size * nmemb; 
>> +}
>> +
>> +#define CHAR_T wchar_t
>> +#define W(o) L##o
>> +#define OPEN_MEMSTREAM open_wmemstream
>> +#define PRINTF wprintf
>> +#define FWRITE fwwrite
>> +#define FPUTC  fputwc
>> +#define STRCMP wcscmp
>> +
>> +#include "tst-memstream3.c"
>> diff --git a/libio/wmemstream.c b/libio/wmemstream.c
>> index bf2a50b..fd01be0 100644
>> --- a/libio/wmemstream.c
>> +++ b/libio/wmemstream.c
>> @@ -112,8 +112,6 @@ _IO_wmem_sync (_IO_FILE *fp)
>>        _IO_wstr_overflow (fp, '\0');
>>        --fp->_wide_data->_IO_write_ptr;
>>      }
>> -  else
>> -    *fp->_wide_data->_IO_write_ptr = '\0';
>>  
>>    *mp->bufloc = fp->_wide_data->_IO_write_base;
>>    *mp->sizeloc = (fp->_wide_data->_IO_write_ptr
>> diff --git a/libio/wstrops.c b/libio/wstrops.c
>> index 09fa543..0b2bec3 100644
>> --- a/libio/wstrops.c
>> +++ b/libio/wstrops.c
>> @@ -169,7 +169,7 @@ _IO_wstr_count (_IO_FILE *fp)
>>  static int
>>  enlarge_userbuf (_IO_FILE *fp, _IO_off64_t offset, int reading)
>>  {
>> -  if ((_IO_ssize_t) offset <= _IO_blen (fp))
>> +  if ((_IO_ssize_t) offset <= _IO_wblen (fp))
>>      return 0;
>>  
>>    struct _IO_wide_data *wd = fp->_wide_data;
>> @@ -235,6 +235,22 @@ enlarge_userbuf (_IO_FILE *fp, _IO_off64_t offset, int reading)
>>    return 0;
>>  }
>>  
>> +static void
>> +_IO_wstr_switch_to_get_mode (_IO_FILE *fp)
>> +{
>> +  if (_IO_in_backup (fp))
>> +    fp->_wide_data->_IO_read_base = fp->_wide_data->_IO_backup_base;
>> +  else
>> +    {
>> +      fp->_wide_data->_IO_read_base = fp->_wide_data->_IO_buf_base;
>> +      if (fp->_wide_data->_IO_write_ptr > fp->_wide_data->_IO_read_end)
>> +        fp->_wide_data->_IO_read_end = fp->_wide_data->_IO_write_ptr;
>> +    }
>> +  fp->_wide_data->_IO_read_ptr = fp->_wide_data->_IO_write_ptr;
>> +  fp->_wide_data->_IO_read_end = fp->_wide_data->_IO_write_ptr;
>> +
>> +  fp->_flags &= ~_IO_CURRENTLY_PUTTING;
>> +}
>>  
>>  _IO_off64_t
>>  _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>> @@ -244,15 +260,16 @@ _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>>    if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET))
>>      mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT);
>>  
>> +  bool was_writing = (fp->_wide_data->_IO_write_ptr >
>> +			fp->_wide_data->_IO_write_base
>> +		     || _IO_in_put_mode (fp));
>> +  if (was_writing)
>> +    _IO_wstr_switch_to_get_mode (fp);
>> +
>>    if (mode == 0)
>>      {
>> -      /* Don't move any pointers. But there is no clear indication what
>> -	 mode FP is in. Let's guess. */
>> -      if (fp->_IO_file_flags & _IO_NO_WRITES)
>> -        new_pos = fp->_wide_data->_IO_read_ptr - fp->_wide_data->_IO_read_base;
>> -      else
>> -        new_pos = (fp->_wide_data->_IO_write_ptr
>> -		   - fp->_wide_data->_IO_write_base);
>> +      new_pos = (fp->_wide_data->_IO_write_ptr
>> +		 - fp->_wide_data->_IO_write_base);
>>      }
>>    else
>>      {
>> @@ -262,25 +279,32 @@ _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>>        /* Move the get pointer, if requested. */
>>        if (mode & _IOS_INPUT)
>>  	{
>> +	  _IO_ssize_t base;
>>  	  switch (dir)
>>  	    {
>> -	    case _IO_seek_end:
>> -	      offset += cur_size;
>> +	    case _IO_seek_set:
>> +	      base = 0;
>>  	      break;
>>  	    case _IO_seek_cur:
>> -	      offset += (fp->_wide_data->_IO_read_ptr
>> -			 - fp->_wide_data->_IO_read_base);
>> +	      base = (fp->_wide_data->_IO_read_ptr
>> +		     - fp->_wide_data->_IO_read_base);
>>  	      break;
>> -	    default: /* case _IO_seek_set: */
>> +	    default: /* case _IO_seek_end: */
>> +	      base = cur_size;
>>  	      break;
>>  	    }
>> -	  if (offset < 0)
>> -	    return EOF;
>> -	  if ((_IO_ssize_t) offset > cur_size
>> -	      && enlarge_userbuf (fp, offset, 1) != 0)
>> +	  _IO_ssize_t maxval = SSIZE_MAX/sizeof (wchar_t) - base;
>> +	  if (offset < -base || offset > maxval)
>> +	    {
>> +	      __set_errno (EINVAL);
>> +	      return EOF;
>> +	    }
>> +	  base += offset;
>> +	  if (base > cur_size
>> +	      && enlarge_userbuf (fp, base, 1) != 0)
>>  	    return EOF;
>>  	  fp->_wide_data->_IO_read_ptr = (fp->_wide_data->_IO_read_base
>> -					  + offset);
>> +					  + base);
>>  	  fp->_wide_data->_IO_read_end = (fp->_wide_data->_IO_read_base
>>  					  + cur_size);
>>  	  new_pos = offset;
>> @@ -289,26 +313,33 @@ _IO_wstr_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>>        /* Move the put pointer, if requested. */
>>        if (mode & _IOS_OUTPUT)
>>  	{
>> +	  _IO_ssize_t base;
>>  	  switch (dir)
>>  	    {
>> -	    case _IO_seek_end:
>> -	      offset += cur_size;
>> +	    case _IO_seek_set:
>> +	      base = 0;
>>  	      break;
>>  	    case _IO_seek_cur:
>> -	      offset += (fp->_wide_data->_IO_write_ptr
>> -			 - fp->_wide_data->_IO_write_base);
>> +	      base = (fp->_wide_data->_IO_write_ptr
>> +		     - fp->_wide_data->_IO_write_base);
>>  	      break;
>> -	    default: /* case _IO_seek_set: */
>> +	    default: /* case _IO_seek_end: */
>> +	      base = cur_size;
>>  	      break;
>>  	    }
>> -	  if (offset < 0)
>> -	    return EOF;
>> -	  if ((_IO_ssize_t) offset > cur_size
>> -	      && enlarge_userbuf (fp, offset, 0) != 0)
>> +	  _IO_ssize_t maxval = SSIZE_MAX/sizeof (wchar_t) - base;
>> +	  if (offset < -base || offset > maxval)
>> +	    {
>> +	      __set_errno (EINVAL);
>> +	      return EOF;
>> +	    }
>> +	  base += offset;
>> +	  if (base > cur_size
>> +	      && enlarge_userbuf (fp, base, 0) != 0)
>>  	    return EOF;
>>  	  fp->_wide_data->_IO_write_ptr = (fp->_wide_data->_IO_write_base
>> -					   + offset);
>> -	  new_pos = offset;
>> +					   + base);
>> +	  new_pos = base;
>>  	}
>>      }
>>    return new_pos;
>> diff --git a/manual/examples/memstrm.c b/manual/examples/memstrm.c
>> index 0d443b1..5701ba1 100644
>> --- a/manual/examples/memstrm.c
>> +++ b/manual/examples/memstrm.c
>> @@ -27,10 +27,10 @@ main (void)
>>    stream = open_memstream (&bp, &size);
>>    fprintf (stream, "hello");
>>    fflush (stream);
>> -  printf ("buf = `%s', size = %d\n", bp, size);
>> +  printf ("buf = `%s', size = %zu\n", bp, size);
>>    fprintf (stream, ", world");
>>    fclose (stream);
>> -  printf ("buf = `%s', size = %d\n", bp, size);
>> +  printf ("buf = `%s', size = %zu\n", bp, size);
>>  
>>    return 0;
>>  }
>>

Ok.

Andreas.

tst-memstream3.c:32:17: error: implicit declaration of function 'strcmp' [-Werror=implicit-function-declaration]
 # define STRCMP strcmp
                 ^
tst-memstream3.c:96:7: note: in expansion of macro 'STRCMP'
   if (STRCMP (buf, W("b")) != 0)
       ^~~~~~

Andreas.

Unfortunately, I did not see it on the environments I have tested
(basically ubuntu 16).  I pushed a obvious fix for this as edbdf87,
thanks for reporting it.

On 01/10/2016 06:17, Andreas Schwab wrote:
> tst-memstream3.c:32:17: error: implicit declaration of function 'strcmp' [-Werror=implicit-function-declaration]
>  # define STRCMP strcmp
>                  ^
> tst-memstream3.c:96:7: note: in expansion of macro 'STRCMP'
>    if (STRCMP (buf, W("b")) != 0)
>        ^~~~~~
> 
> Andreas.
>

On 08/08/2016 10:43 PM, Adhemerval Zanella wrote:
> @@ -239,14 +254,14 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>     if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET))
>       mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT);
>   
> +  bool was_writing = (fp->_IO_write_ptr > fp->_IO_write_base
> +		     || _IO_in_put_mode (fp));
> +  if (was_writing)
> +    _IO_str_switch_to_get_mode (fp);

This patch breaks backwards compatibility with applications which call 
_IO_str_seekoff directly.  This is an exported function and it was 
originally intended as a building block for building custom streams, so 
we cannot change what it does just to fit glibc's internal needs, based 
on how the function is called from within glibc.

But if we apply this standard of backwards compatibility, we cannot make 
*any* changes to libio (including important security hardening) without 
copying most of the code first.  We have no tests which check the 
extended API behavior, and the interface is very much under-documented, too.

What should we do here?

I'm leaning towards a clean break: Stop installing <libio.h>.  Remove 
all symbols related to external vtable support (i.e., an ABI break, so 
that affected programs fail in a clean manner).  Do this now, and 
revisit it for glibc 2.27 if someone actually has an application that 
breaks due to this.

For the old inlined putc_unlocked implementation which directly poked at 
struct _IO_FILE, I suggest we write tests and keep compatibility.

Comments?

Thanks,
Florian

On Wed, Apr 19, 2017 at 11:17:21AM +0200, Florian Weimer wrote:
[...]
> I'm leaning towards a clean break: Stop installing <libio.h>.

You must be joking.  This would definitely break libzio.

On Wed, Apr 19, 2017 at 5:17 AM, Florian Weimer <fweimer@redhat.com> wrote:
> I'm leaning towards a clean break: Stop installing <libio.h>.  Remove all
> symbols related to external vtable support (i.e., an ABI break, so that
> affected programs fail in a clean manner).  Do this now, and revisit it for
> glibc 2.27 if someone actually has an application that breaks due to this.

I had been thinking about proposing a patch to stop installing
libio.h, myself.  It definitely is getting in the way of forward
progress.

However, stdio in general could use a great deal of revision, and we
don't want to break compatibility several releases in a row.  Maybe it
makes more sense to start in on a "revise stdio" project on a branch,
but not merge any breaking changes until it's completely ready to go?
Meanwhile, on trunk, we could make stdio.h stop including libio.h and
add a deprecation warning to libio.h -- that should be enough to flush
out applications that still need it, and find out what exactly they
need.

(I wish I had time to help with a stdio overhaul, but realistically I don't.)

zw

On Wed, Apr 19, 2017 at 8:35 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Wed, Apr 19, 2017 at 11:17:21AM +0200, Florian Weimer wrote:
> [...]
>> I'm leaning towards a clean break: Stop installing <libio.h>.
>
> You must be joking.  This would definitely break libzio.

Never heard of it. What applications use it?

zw

On 04/19/2017 02:35 PM, Dmitry V. Levin wrote:
> On Wed, Apr 19, 2017 at 11:17:21AM +0200, Florian Weimer wrote:
> [...]
>> I'm leaning towards a clean break: Stop installing <libio.h>.
> 
> You must be joking.  This would definitely break libzio.

I'm not.

I just tried, and libzio 1.04 compiles fine with !HAVE_LIBIO_H.  It 
doesn't define its own vtables, either, nor does it internal _IO_* symbols.

Florian

On Wed, Apr 19, 2017 at 08:52:55AM -0400, Zack Weinberg wrote:
> On Wed, Apr 19, 2017 at 8:35 AM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> > On Wed, Apr 19, 2017 at 11:17:21AM +0200, Florian Weimer wrote:
> > [...]
> >> I'm leaning towards a clean break: Stop installing <libio.h>.
> >
> > You must be joking.  This would definitely break libzio.
> 
> Never heard of it. What applications use it?

A slightly patched version (git://git.altlinux.org/gears/l/libzio.git)
is part of our build environment:

$ rpm -e --test libzio
error: Failed dependencies:
	libzio.so.0()(64bit) >= set:feLS is needed by (installed) info-install-6.0-alt3.x86_64
	libzio.so.0()(64bit) >= set:feLS is needed by (installed) patchutils-0.3.4-alt1.x86_64

On Wed, Apr 19, 2017 at 03:01:25PM +0200, Florian Weimer wrote:
> On 04/19/2017 02:35 PM, Dmitry V. Levin wrote:
> > On Wed, Apr 19, 2017 at 11:17:21AM +0200, Florian Weimer wrote:
> > [...]
> >> I'm leaning towards a clean break: Stop installing <libio.h>.
> > 
> > You must be joking.  This would definitely break libzio.
> 
> I'm not.
> 
> I just tried, and libzio 1.04 compiles fine with !HAVE_LIBIO_H.  It 
> doesn't define its own vtables, either, nor does it internal _IO_* symbols.

libzio uses cookie_io_functions_t provided by libio.h;
in case of !HAVE_LIBIO_H it falls back to _IO_cookie_io_functions_t
also defined by libio.h via stdio.h;

If stdio.h continues to provide fopencookie and _IO_cookie_io_functions_t,
this should be enough for libzio.

On 04/19/2017 03:17 PM, Dmitry V. Levin wrote:
> On Wed, Apr 19, 2017 at 03:01:25PM +0200, Florian Weimer wrote:
>> On 04/19/2017 02:35 PM, Dmitry V. Levin wrote:
>>> On Wed, Apr 19, 2017 at 11:17:21AM +0200, Florian Weimer wrote:
>>> [...]
>>>> I'm leaning towards a clean break: Stop installing <libio.h>.
>>>
>>> You must be joking.  This would definitely break libzio.
>>
>> I'm not.
>>
>> I just tried, and libzio 1.04 compiles fine with !HAVE_LIBIO_H.  It
>> doesn't define its own vtables, either, nor does it internal _IO_* symbols.
> 
> libzio uses cookie_io_functions_t provided by libio.h;
> in case of !HAVE_LIBIO_H it falls back to _IO_cookie_io_functions_t
> also defined by libio.h via stdio.h;
> 
> If stdio.h continues to provide fopencookie and _IO_cookie_io_functions_t,
> this should be enough for libzio.

The documented type of the fopencookie argument is 
cookie_io_functions_t, and we will continue to provide that from 
<stdio.h> (if _GNU_SOURCE is defined).

Thanks,
Florian

On 19/04/2017 06:17, Florian Weimer wrote:
> On 08/08/2016 10:43 PM, Adhemerval Zanella wrote:
>> @@ -239,14 +254,14 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>>     if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET))
>>       mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT);
>>   +  bool was_writing = (fp->_IO_write_ptr > fp->_IO_write_base
>> +             || _IO_in_put_mode (fp));
>> +  if (was_writing)
>> +    _IO_str_switch_to_get_mode (fp);
> 
> This patch breaks backwards compatibility with applications which call _IO_str_seekoff directly.  This is an exported function and it was originally intended as a building block for building custom streams, so we cannot change what it does just to fit glibc's internal needs, based on how the function is called from within glibc.
> 
> But if we apply this standard of backwards compatibility, we cannot make *any* changes to libio (including important security hardening) without copying most of the code first.  We have no tests which check the extended API behavior, and the interface is very much under-documented, too.
> 
> What should we do here?

Right, so should we revert the patch, reopen both bugs and rework all libio 
in this case (which might span on multiple releases)? I know that we should
aim for compatibility where applicable, but I think blindly aim for it even
for bug/out of conformance cases adds more maintainer burden that actually
fixes real cases usage. 

For this specific case, the code is clearly buggy when ran a different libc
for a non-specific gnu extension (open_memstream). Should we still provide 
buggy compat interfaces in this case (as we are still aiming to provide)?

> 
> I'm leaning towards a clean break: Stop installing <libio.h>.  Remove all symbols related to external vtable support (i.e., an ABI break, so that affected programs fail in a clean manner).  Do this now, and revisit it for glibc 2.27 if someone actually has an application that breaks due to this.
> 
> For the old inlined putc_unlocked implementation which directly poked at struct _IO_FILE, I suggest we write tests and keep compatibility.
> 
> Comments?
> 
> Thanks,
> Florian

On 04/19/2017 04:48 PM, Adhemerval Zanella wrote:
> 
> 
> On 19/04/2017 06:17, Florian Weimer wrote:
>> On 08/08/2016 10:43 PM, Adhemerval Zanella wrote:
>>> @@ -239,14 +254,14 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>>>      if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET))
>>>        mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT);
>>>    +  bool was_writing = (fp->_IO_write_ptr > fp->_IO_write_base
>>> +             || _IO_in_put_mode (fp));
>>> +  if (was_writing)
>>> +    _IO_str_switch_to_get_mode (fp);
>>
>> This patch breaks backwards compatibility with applications which call _IO_str_seekoff directly.  This is an exported function and it was originally intended as a building block for building custom streams, so we cannot change what it does just to fit glibc's internal needs, based on how the function is called from within glibc.
>>
>> But if we apply this standard of backwards compatibility, we cannot make *any* changes to libio (including important security hardening) without copying most of the code first.  We have no tests which check the extended API behavior, and the interface is very much under-documented, too.
>>
>> What should we do here?
> 
> Right, so should we revert the patch, reopen both bugs and rework all libio
> in this case (which might span on multiple releases)? I know that we should
> aim for compatibility where applicable, but I think blindly aim for it even
> for bug/out of conformance cases adds more maintainer burden that actually
> fixes real cases usage.
> 
> For this specific case, the code is clearly buggy when ran a different libc
> for a non-specific gnu extension (open_memstream). Should we still provide
> buggy compat interfaces in this case (as we are still aiming to provide)?

Sorry, I wasn't clear.  The buggy interface is open_memstream.  Fixing 
that is completely fine, no compatibility symbol is required.  But 
_IO_str_seekoff is a completely different, allegedly public interface, 
and the existing callers most expect some concrete behavior from it.

A hypothetical example of the same scenario: posix_fallocate used to 
have a bug that it did not work on O_APPEND descriptors because pwrite64 
ignored the offset for them, as required by POSIX.  We could have fixed 
that by changing pwrite64 not to ignore the write offset, but that's of 
course bogus because the specified semantics for pwrite64 require 
different behavior.

What I'm trying to say is that similar, but undocumented requirements 
might well exist for the _IO_str_seekoff function.  Hence my original 
comment that we'd need to make a copy first, fix the copy, and adjust 
internal callers to use the copy, leaving the original implementation alone.

Things get worse once we start changing struct definitions.  Then we 
basically have to duplicate everything that depends on those structs.

I think that's not a good use of our time because it is very unlikely 
that there are any applications left which use these interfaces. 
Instead, I suggest that we make explicit that the internal libio 
interfaces are unsupported (because they are unsupportable), and remove 
them from the ABI, so that people who have those old binaries get clear 
dynamic linker failures instead of corrupted data.

Thanks,
Florian

On 19/04/2017 12:02, Florian Weimer wrote:
> On 04/19/2017 04:48 PM, Adhemerval Zanella wrote:
>>
>>
>> On 19/04/2017 06:17, Florian Weimer wrote:
>>> On 08/08/2016 10:43 PM, Adhemerval Zanella wrote:
>>>> @@ -239,14 +254,14 @@ _IO_str_seekoff (_IO_FILE *fp, _IO_off64_t offset, int dir, int mode)
>>>>      if (mode == 0 && (fp->_flags & _IO_TIED_PUT_GET))
>>>>        mode = (fp->_flags & _IO_CURRENTLY_PUTTING ? _IOS_OUTPUT : _IOS_INPUT);
>>>>    +  bool was_writing = (fp->_IO_write_ptr > fp->_IO_write_base
>>>> +             || _IO_in_put_mode (fp));
>>>> +  if (was_writing)
>>>> +    _IO_str_switch_to_get_mode (fp);
>>>
>>> This patch breaks backwards compatibility with applications which call _IO_str_seekoff directly.  This is an exported function and it was originally intended as a building block for building custom streams, so we cannot change what it does just to fit glibc's internal needs, based on how the function is called from within glibc.
>>>
>>> But if we apply this standard of backwards compatibility, we cannot make *any* changes to libio (including important security hardening) without copying most of the code first.  We have no tests which check the extended API behavior, and the interface is very much under-documented, too.
>>>
>>> What should we do here?
>>
>> Right, so should we revert the patch, reopen both bugs and rework all libio
>> in this case (which might span on multiple releases)? I know that we should
>> aim for compatibility where applicable, but I think blindly aim for it even
>> for bug/out of conformance cases adds more maintainer burden that actually
>> fixes real cases usage.
>>
>> For this specific case, the code is clearly buggy when ran a different libc
>> for a non-specific gnu extension (open_memstream). Should we still provide
>> buggy compat interfaces in this case (as we are still aiming to provide)?
> 
> Sorry, I wasn't clear.  The buggy interface is open_memstream.  Fixing that is completely fine, no compatibility symbol is required.  But _IO_str_seekoff is a completely different, allegedly public interface, and the existing callers most expect some concrete behavior from it.
> 
> A hypothetical example of the same scenario: posix_fallocate used to have a bug that it did not work on O_APPEND descriptors because pwrite64 ignored the offset for them, as required by POSIX.  We could have fixed that by changing pwrite64 not to ignore the write offset, but that's of course bogus because the specified semantics for pwrite64 require different behavior.
> 
> What I'm trying to say is that similar, but undocumented requirements might well exist for the _IO_str_seekoff function.  Hence my original comment that we'd need to make a copy first, fix the copy, and adjust internal callers to use the copy, leaving the original implementation alone.
> 
> Things get worse once we start changing struct definitions.  Then we basically have to duplicate everything that depends on those structs.
> 
> I think that's not a good use of our time because it is very unlikely that there are any applications left which use these interfaces. Instead, I suggest that we make explicit that the internal libio interfaces are unsupported (because they are unsupportable), and remove them from the ABI, so that people who have those old binaries get clear dynamic linker failures instead of corrupted data.

This seems a more reasonable approach, I realised it was about _IO_str_seekoff
exposure right after send the email (sorry about that noise).