Fix offset computation for append+ mode on switching from read (BZ #16724)

The offset computation in write mode uses the fact that _IO_read_end
is kept in sync with the external file offset.  This however is not
true when O_APPEND is in effect since switching to write mode ought to
send the external file offset to the end of file without making the
necessary adjustment to _IO_read_end.

Hence in append mode, offset computation when writing should only
consider the effect of unflushed writes, i.e. from _IO_write_base to
_IO_write_ptr.

The wiki has a detailed document that describes the rationale for
offsets returned by ftell in various conditions:

https://sourceware.org/glibc/wiki/File%20offsets%20in%20a%20stdio%20stream%20and%20ftell

6 files changed, 207 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index e65407cf4b..1c2aff1191 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2014-05-27  Siddhesh Poyarekar  <siddhesh@redhat.com>
+
+	[BZ #16724]
+	* libio/tst-ftell-append.c: New test case.
+	* libio/Makefile (tests): Add test case.
+	* libio/fileops.c (do_ftell): Don't trust _IO_read_end when in
+	append mode.
+	* libio/wfileops.c (do_ftell_wide): Likewise.
+
 2014-05-26  Adhemerval Zanella  <azanella@linux.vnet.ibm.com>
 
 	* sysdeps/powerpc/fpu/libm-test-ulps: Update.
diff --git a/NEWS b/NEWS
index 64d2fbb915..331601cad9 100644
--- a/NEWS
+++ b/NEWS
@@ -14,11 +14,11 @@ Version 2.20
   16516, 16532, 16545, 16564, 16574, 16599, 16600, 16609, 16610, 16611,
   16613, 16619, 16623, 16629, 16632, 16634, 16639, 16642, 16648, 16649,
   16670, 16674, 16677, 16680, 16683, 16689, 16695, 16701, 16706, 16707,
-  16712, 16713, 16714, 16731, 16739, 16740, 16743, 16754, 16758, 16759,
-  16760, 16770, 16786, 16789, 16791, 16796, 16799, 16800, 16815, 16823,
-  16824, 16831, 16838, 16849, 16854, 16876, 16877, 16878, 16885, 16888,
-  16890, 16912, 16915, 16916, 16917, 16922, 16927, 16928, 16932, 16943,
-  16958, 16966, 16967, 16965, 16977, 16978, 16984.
+  16712, 16713, 16714, 16724, 16731, 16739, 16740, 16743, 16754, 16758,
+  16759, 16760, 16770, 16786, 16789, 16791, 16796, 16799, 16800, 16815,
+  16823, 16824, 16831, 16838, 16849, 16854, 16876, 16877, 16878, 16885,
+  16888, 16890, 16912, 16915, 16916, 16917, 16922, 16927, 16928, 16932,
+  16943, 16958, 16966, 16967, 16965, 16977, 16978, 16984.
 
 * The minimum Linux kernel version that this version of the GNU C Library
   can be used with is 2.6.32.
diff --git a/libio/Makefile b/libio/Makefile
index cca03454fc..b324ccc519 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -60,7 +60,8 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
 	tst-wmemstream1 tst-wmemstream2 \
 	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-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \
+	tst-ftell-append
 ifeq (yes,$(build-shared))
 # Add test-fopenloc only if shared library is enabled since it depends on
 # shared localedata objects.
@@ -160,6 +161,7 @@ tst-fgetwc-ENV = LOCPATH=$(common-objpfx)localedata
 tst-fseek-ENV = LOCPATH=$(common-objpfx)localedata
 tst-ftell-partial-wide-ENV = LOCPATH=$(common-objpfx)localedata
 tst-ftell-active-handler-ENV = LOCPATH=$(common-objpfx)localedata
+tst-ftell-append-ENV = LOCPATH=$(common-objpfx)localedata
 
 generated += tst-fopenloc.mtrace tst-fopenloc.check
 
diff --git a/libio/fileops.c b/libio/fileops.c
index cf68dbfe52..204cfeaa35 100644
--- a/libio/fileops.c
+++ b/libio/fileops.c
@@ -91,7 +91,9 @@ extern struct __gconv_trans_data __libio_translit attribute_hidden;
 
    The position in the buffer that corresponds to the position
    in external file system is normally _IO_read_end, except in putback
-   mode, when it is _IO_save_end.
+   mode, when it is _IO_save_end and also when the file is in append mode,
+   since switching from read to write mode automatically sends the position in
+   the external file system to the end of file.
    If the field _fb._offset is >= 0, it gives the offset in
    the file as a whole corresponding to eGptr(). (?)
 
@@ -966,6 +968,14 @@ do_ftell (_IO_FILE *fp)
       /* Adjust for unflushed data.  */
       if (!was_writing)
 	offset -= fp->_IO_read_end - fp->_IO_read_ptr;
+      /* We don't trust _IO_read_end to represent the current file offset when
+	 writing in append mode because the value would have to be shifted to
+	 the end of the file during a flush.  Use the write base instead, along
+	 with the new offset we got above when we did a seek to the end of the
+	 file.  */
+      else if (append_mode)
+	offset += fp->_IO_write_ptr - fp->_IO_write_base;
+      /* For all other modes, _IO_read_end represents the file offset.  */
       else
 	offset += fp->_IO_write_ptr - fp->_IO_read_end;
     }
diff --git a/libio/tst-ftell-append.c b/libio/tst-ftell-append.c
new file mode 100644
index 0000000000..604dc0342c
--- /dev/null
+++ b/libio/tst-ftell-append.c
@@ -0,0 +1,169 @@
+/* Verify that ftell returns the correct value after a read and a write on a
+   file opened in a+ mode.
+   Copyright (C) 2014 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 <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <unistd.h>
+#include <locale.h>
+#include <wchar.h>
+
+/* data points to either char_data or wide_data, depending on whether we're
+   testing regular file mode or wide mode respectively.  Similarly,
+   fputs_func points to either fputs or fputws.  data_len keeps track of the
+   length of the current data and file_len maintains the current file
+   length.  */
+#define BUF_LEN 4
+static void *buf;
+static char char_buf[BUF_LEN];
+static wchar_t wide_buf[BUF_LEN];
+static const void *data;
+static const char *char_data = "abcdefghijklmnopqrstuvwxyz";
+static const wchar_t *wide_data = L"abcdefghijklmnopqrstuvwxyz";
+static size_t data_len;
+static size_t file_len;
+
+typedef int (*fputs_func_t) (const void *data, FILE *fp);
+fputs_func_t fputs_func;
+
+typedef void *(*fgets_func_t) (void *s, int size, FILE *stream);
+fgets_func_t fgets_func;
+
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
+static FILE *
+init_file (const char *filename)
+{
+  FILE *fp = fopen (filename, "w");
+  if (fp == NULL)
+    {
+      printf ("fopen: %m\n");
+      return NULL;
+    }
+
+  int written = fputs_func (data, fp);
+
+  if (written == EOF)
+    {
+      printf ("fputs failed to write data\n");
+      fclose (fp);
+      return NULL;
+    }
+
+  file_len = data_len;
+
+  fclose (fp);
+
+  fp = fopen (filename, "a+");
+  if (fp == NULL)
+    {
+      printf ("fopen(a+): %m\n");
+      return NULL;
+    }
+
+  return fp;
+}
+
+static int
+do_one_test (const char *filename)
+{
+  FILE *fp = init_file (filename);
+
+  if (fp == NULL)
+    return 1;
+
+  void *ret = fgets_func (buf, BUF_LEN, fp);
+
+  if (ret == NULL)
+    {
+      printf ("read failed: %m\n");
+      fclose (fp);
+      return 1;
+    }
+
+  int written = fputs_func (data, fp);
+
+  if (written == EOF)
+    {
+      printf ("fputs failed to write data\n");
+      fclose (fp);
+      return 1;
+    }
+
+  file_len += data_len;
+
+  long off = ftell (fp);
+
+  if (off != file_len)
+    {
+      printf ("Incorrect offset %ld, expected %zu\n", off, file_len);
+      fclose (fp);
+      return 1;
+    }
+  else
+    printf ("Correct offset %ld after write.\n", off);
+
+  return 0;
+}
+
+/* Run the tests for regular files and wide mode files.  */
+static int
+do_test (void)
+{
+  int ret = 0;
+  char *filename;
+  int fd = create_temp_file ("tst-ftell-append-tmp.", &filename);
+
+  if (fd == -1)
+    {
+      printf ("create_temp_file: %m\n");
+      return 1;
+    }
+
+  close (fd);
+
+  /* Tests for regular files.  */
+  puts ("Regular mode:");
+  fputs_func = (fputs_func_t) fputs;
+  fgets_func = (fgets_func_t) fgets;
+  data = char_data;
+  buf = char_buf;
+  data_len = strlen (char_data);
+  ret |= do_one_test (filename);
+
+  /* Tests for wide files.  */
+  puts ("Wide mode:");
+  if (setlocale (LC_ALL, "en_US.UTF-8") == NULL)
+    {
+      printf ("Cannot set en_US.UTF-8 locale.\n");
+      return 1;
+    }
+  fputs_func = (fputs_func_t) fputws;
+  fgets_func = (fgets_func_t) fgetws;
+  data = wide_data;
+  buf = wide_buf;
+  data_len = wcslen (wide_data);
+  ret |= do_one_test (filename);
+
+  return ret;
+}
diff --git a/libio/wfileops.c b/libio/wfileops.c
index 3199861b4d..f123add354 100644
--- a/libio/wfileops.c
+++ b/libio/wfileops.c
@@ -713,9 +713,16 @@ do_ftell_wide (_IO_FILE *fp)
 	      offset += outstop - out;
 	    }
 
-	  /* _IO_read_end coincides with fp._offset, so the actual file
-	     position is fp._offset - (_IO_read_end - new_write_ptr).  */
-	  offset -= fp->_IO_read_end - fp->_IO_write_ptr;
+	  /* We don't trust _IO_read_end to represent the current file offset
+	     when writing in append mode because the value would have to be
+	     shifted to the end of the file during a flush.  Use the write base
+	     instead, along with the new offset we got above when we did a seek
+	     to the end of the file.  */
+	  if (append_mode)
+	    offset += fp->_IO_write_ptr - fp->_IO_write_base;
+	  /* For all other modes, _IO_read_end represents the file offset.  */
+	  else
+	    offset += fp->_IO_write_ptr - fp->_IO_read_end;
 	}
     }