This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH 1/2] Move offset to end of file when fdopen is in "a" mode (#16532)


Hi,

Unlike fopen, fdopen fails to move the offset in the file descriptor
to the end of file when the file is opened in append mode (a).  This
results in ftell{,o} incorrectly assuming that the current offset is
at the beginning of file and returns an incorrect result.  This was
exposed due to commit adb26fae.

I have avoided modifying _IO_file_attach on purpose since it is part
of the public ABI and have instead made a modified copy in the form of
a static function.  I didn't bother modifying vdprintf since it is not
affected by this.

The change also includes a test case to check this for fdopen and
fopen.  Verified that it does not cause any regressions in the
testsuite.

OK to commit?

Siddhesh


	[BZ #16532]
	* libio/iofdopen.c (file_attach_fd): New function.
	(_IO_new_fdopen): Use it.
	* libio/tst-fopen-append-update.c: New test case.
	* libio/Makefile (tests): Add it.


---
 libio/Makefile                  |   2 +-
 libio/iofdopen.c                |  36 +++++++++-
 libio/tst-fopen-append-update.c | 147 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 182 insertions(+), 3 deletions(-)
 create mode 100644 libio/tst-fopen-append-update.c

diff --git a/libio/Makefile b/libio/Makefile
index 747a779..640b475 100644
--- a/libio/Makefile
+++ b/libio/Makefile
@@ -60,7 +60,7 @@ 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-fwrite-error tst-ftell-partial-wide tst-fopen-append-update
 ifeq (yes,$(build-shared))
 # Add test-fopenloc only if shared library is enabled since it depends on
 # shared localedata objects.
diff --git a/libio/iofdopen.c b/libio/iofdopen.c
index 066ff19..b214ea6 100644
--- a/libio/iofdopen.c
+++ b/libio/iofdopen.c
@@ -40,6 +40,38 @@
 #endif
 #endif
 
+/* Attach the FD to the FILE object FP.  If READ_WRITE indicates an append
+   mode, seek to the end of the file instead of seeking to the current
+   position.  */
+static _IO_FILE *
+file_attach_fd (_IO_FILE *fp, int fd, int read_write)
+{
+  int dir = _IO_seek_cur;
+
+  if (_IO_file_is_open (fp))
+    return NULL;
+
+  fp->_fileno = fd;
+  fp->_flags &= ~(_IO_NO_READS+_IO_NO_WRITES);
+  fp->_flags |= _IO_DELETE_DONT_CLOSE;
+  /* Get the current position of the file. */
+  /* We have to do that since that may be junk. */
+  fp->_offset = _IO_pos_BAD;
+  int save_errno = errno;
+
+  if ((read_write & _IO_IS_APPENDING) && (read_write & _IO_NO_READS))
+    dir = _IO_seek_end;
+
+  _IO_off64_t ret = _IO_SEEKOFF (fp, (_IO_off64_t) 0, dir,
+				 _IOS_INPUT | _IOS_OUTPUT);
+
+  if (ret == _IO_pos_BAD && errno != ESPIPE)
+    return NULL;
+
+  __set_errno (save_errno);
+  return fp;
+}
+
 _IO_FILE *
 _IO_new_fdopen (fd, mode)
      int fd;
@@ -143,7 +175,7 @@ _IO_new_fdopen (fd, mode)
 #endif
   /* Set up initially to use the `maybe_mmap' jump tables rather than using
      __fopen_maybe_mmap to do it, because we need them in place before we
-     call _IO_file_attach or else it will allocate a buffer immediately.  */
+     call file_attach_fd or else it will allocate a buffer immediately.  */
   _IO_no_init (&new_f->fp.file, 0, 0, &new_f->wd,
 #ifdef _G_HAVE_MMAP
 	       (use_mmap && (read_write & _IO_NO_WRITES))
@@ -159,7 +191,7 @@ _IO_new_fdopen (fd, mode)
 #if  !_IO_UNIFIED_JUMPTABLES
   new_f->fp.vtable = NULL;
 #endif
-  if (_IO_file_attach ((_IO_FILE *) &new_f->fp, fd) == NULL)
+  if (file_attach_fd ((_IO_FILE *) &new_f->fp, fd, read_write) == NULL)
     {
       _IO_setb (&new_f->fp.file, NULL, NULL, 0);
       _IO_un_link (&new_f->fp);
diff --git a/libio/tst-fopen-append-update.c b/libio/tst-fopen-append-update.c
new file mode 100644
index 0000000..83d7744
--- /dev/null
+++ b/libio/tst-fopen-append-update.c
@@ -0,0 +1,147 @@
+/* Verify that ftell returns the correct value when a file is opened in
+   append (a) and append-update (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 <fcntl.h>
+
+static int do_test (void);
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+
+static const char *data = "abcdef";
+
+static int
+do_one_test (const char *filename, int count, bool do_fopen)
+{
+  FILE *fp = NULL;
+  int ret = 0;
+
+  if (do_fopen)
+    {
+      fp = fopen (filename, "a");
+      if (fp == NULL)
+	{
+	  printf ("fopen[%d]: %m\n", count);
+	  ret = 1;
+	  goto out;
+	}
+    }
+  else
+    {
+      int fd = open (filename, O_WRONLY, 0);
+
+      if (fd == -1)
+	{
+	  printf ("Failed to open temp file: %m\n");
+	  ret = 1;
+	  goto out;
+	}
+
+      fp = fdopen (fd, "a");
+      if (fp == NULL)
+	{
+	  printf ("fdopen[%d]: %m\n", count);
+	  ret = 1;
+	  close (fd);
+	  goto out;
+	}
+    }
+
+  /* Write some data.  */
+  size_t written = fwrite (data, 1, strlen (data), fp);
+
+  if (written != strlen (data))
+    {
+      printf ("fwrite[1] failed to write all data\n");
+      ret = 1;
+      goto out;
+    }
+
+  /* Verify that the offset points to the end of the file.  */
+  long offset = ftell (fp);
+
+  if (offset != count * strlen (data))
+    {
+      printf ("Incorrect offset.  Expected %zu, but got %ld\n",
+	      (count * strlen (data)), offset);
+
+      ret = 1;
+      goto out;
+    }
+
+  printf ("Test 1 succeeded for %s: offset = %ld\n",
+	  do_fopen ? "fopen" : "fdopen", offset);
+
+out:
+  if (fp)
+    fclose (fp);
+  return ret;
+}
+
+static int
+do_test (void)
+{
+  int ret = 0;
+  FILE *fp = NULL;
+  char *filename;
+  size_t written;
+  int fd = create_temp_file ("tst-fseek-wide-partial.out", &filename);
+
+  if (fd == -1)
+    {
+      printf ("create_temp_file: %m\n");
+      return 1;
+    }
+
+  fp = fdopen (fd, "w");
+  if (fp == NULL)
+    {
+      printf ("fdopen[0]: %m\n");
+      close (fd);
+      return 1;
+    }
+
+  written = fwrite (data, 1, strlen (data), fp);
+
+  if (written != strlen (data))
+    {
+      printf ("fwrite[1] failed to write all data\n");
+      ret = 1;
+    }
+
+  fclose (fp);
+  if (ret)
+    return ret;
+
+  /* Counter to keep track of what offset to expect in the file and also to
+     enumerate the tests.  */
+  int i = 1;
+
+  /* In append mode (a), fopen and fdopen should move its offset to the end of
+     the file.  */
+  ret |= do_one_test (filename, ++i, false);
+  ret |= do_one_test (filename, ++i, true);
+
+  return ret;
+}
-- 
1.8.3.1


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]