svn commit: r1808456 - in /apr/apr/trunk: CHANGES file_io/win32/open.c test/testfile.c

[ko...@apache.org]

Author: kotkov
Date: Fri Sep 15 13:25:08 2017
New Revision: 1808456

URL: http://svn.apache.org/viewvc?rev=1808456&view=rev
Log:
Win32: Don't seek to the end when opening files with APR_FOPEN_APPEND.

Apparently, this is a leftover from the very first version of handling file
appends (https://svn.apache.org/r59449) that performed a single seek to
the file's end when opening it and did not support proper atomic appends
with multiple process or threads writing to the same file.

Since then, such atomic appends have been implemented, but the seek
was not removed.  It can cause unexpected behavior when reading from
a file opened with APR_FOPEN_APPEND, assuming no writes happened
to the file.  In this case, as there have been no writes, the file offset
should not be repositioned and reading should start from the beginning of
the file.  However, due to the unwanted seek during open, the actual reading
would instead start from the file's end and cause an unexpected EOF.

* file_io/win32/open.c
  (apr_file_open): Don't seek to the file's end when the file is
   opened with APR_FOPEN_APPEND.

* test/testfile.c
  (test_append_read): New test.
  (testfile): Run the new test.

* CHANGES: Add changelog entry.

Modified:
    apr/apr/trunk/CHANGES
    apr/apr/trunk/file_io/win32/open.c
    apr/apr/trunk/test/testfile.c

Modified: apr/apr/trunk/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/trunk/CHANGES?rev=1808456&r1=1808455&r2=1808456&view=diff
==============================================================================
--- apr/apr/trunk/CHANGES [utf-8] (original)
+++ apr/apr/trunk/CHANGES [utf-8] Fri Sep 15 13:25:08 2017
@@ -1,6 +1,9 @@
                                                      -*- coding: utf-8 -*-
 Changes for APR 2.0.0
 
+  *) Don't seek to the end when opening files with APR_FOPEN_APPEND on Windows.
+     [Evgeny Kotkov <evgeny.kotkov visualsvn.com>]
+
   *) Fix a deadlock when writing to locked files opened with APR_FOPEN_APPEND
      on Windows. PR 50058.
      [Evgeny Kotkov <evgeny.kotkov visualsvn.com>]

Modified: apr/apr/trunk/file_io/win32/open.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/open.c?rev=1808456&r1=1808455&r2=1808456&view=diff
==============================================================================
--- apr/apr/trunk/file_io/win32/open.c (original)
+++ apr/apr/trunk/file_io/win32/open.c Fri Sep 15 13:25:08 2017
@@ -445,7 +445,6 @@ APR_DECLARE(apr_status_t) apr_file_open(
 
     if (flag & APR_FOPEN_APPEND) {
         (*new)->append = 1;
-        SetFilePointer((*new)->filehand, 0, NULL, FILE_END);
     }
     if (flag & APR_FOPEN_BUFFERED) {
         (*new)->buffered = 1;

Modified: apr/apr/trunk/test/testfile.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/test/testfile.c?rev=1808456&r1=1808455&r2=1808456&view=diff
==============================================================================
--- apr/apr/trunk/test/testfile.c (original)
+++ apr/apr/trunk/test/testfile.c Fri Sep 15 13:25:08 2017
@@ -1793,6 +1793,56 @@ static void test_append_locked(abts_case
     apr_file_remove(fname, p);
 }
 
+static void test_append_read(abts_case *tc, void *data)
+{
+    apr_status_t rv;
+    apr_file_t *f;
+    const char *fname = "data/testappend_read.dat";
+    apr_off_t offset;
+    char buf[64];
+
+    apr_file_remove(fname, p);
+
+    /* Create file with contents. */
+    rv = apr_file_open(&f, fname, APR_FOPEN_WRITE | APR_FOPEN_CREATE,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "create file", rv);
+
+    rv = apr_file_puts("abc", f);
+    APR_ASSERT_SUCCESS(tc, "write to file", rv);
+    apr_file_close(f);
+
+    /* Re-open it with APR_FOPEN_APPEND. */
+    rv = apr_file_open(&f, fname,
+                       APR_FOPEN_READ | APR_FOPEN_WRITE | APR_FOPEN_APPEND,
+                       APR_FPROT_OS_DEFAULT, p);
+    APR_ASSERT_SUCCESS(tc, "open file", rv);
+
+    /* Test the initial file offset.  Even though we used APR_FOPEN_APPEND,
+     * the offset should be kept in the beginning of the file until the
+     * first append.  (Previously, the Windows implementation performed
+     * an erroneous seek to the file's end right after opening it.)
+     */
+    offset = 0;
+    rv = apr_file_seek(f, APR_CUR, &offset);
+    APR_ASSERT_SUCCESS(tc, "get file offset", rv);
+    ABTS_INT_EQUAL(tc, 0, (int)offset);
+
+    /* Test reading from the file. */
+    rv = apr_file_gets(buf, sizeof(buf), f);
+    APR_ASSERT_SUCCESS(tc, "read from file", rv);
+    ABTS_STR_EQUAL(tc, "abc", buf);
+
+    /* Test the file offset after reading. */
+    offset = 0;
+    rv = apr_file_seek(f, APR_CUR, &offset);
+    APR_ASSERT_SUCCESS(tc, "get file offset", rv);
+    ABTS_INT_EQUAL(tc, 3, (int)offset);
+
+    apr_file_close(f);
+    apr_file_remove(fname, p);
+}
+
 abts_suite *testfile(abts_suite *suite)
 {
     suite = ADD_SUITE(suite)
@@ -1848,6 +1898,7 @@ abts_suite *testfile(abts_suite *suite)
     abts_run_test(suite, test_write_buffered_spanning_over_bufsize, NULL);
     abts_run_test(suite, test_atomic_append, NULL);
     abts_run_test(suite, test_append_locked, NULL);
+    abts_run_test(suite, test_append_read, NULL);
 
     return suite;
 }