You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by ko...@apache.org on 2023/04/12 12:51:03 UTC
svn commit: r1909088 - in /apr/apr/trunk: CHANGES file_io/win32/open.c test/testfile.c
Author: kotkov
Date: Wed Apr 12 12:51:03 2023
New Revision: 1909088
URL: http://svn.apache.org/viewvc?rev=1909088&view=rev
Log:
Revert r1808456 (Win32: Don't seek to the end when opening files with
APR_FOPEN_APPEND).
While this change fixed an issue where Windows and Unix reported different
offsets after opening files for append, it also caused a regression: for
files opened with APR_FOPEN_APPEND | APR_FOPEN_BUFFERED, flushing their
contents would cause the contents to be written at offset 0, rather than
appended.
This happens because flushes and regular writes use different code paths.
And while regular writes guarantee that an append will happen to the end of
the file, a buffer flush uses a regular WriteFile(), assuming the file pointer
has been properly set before.
To fix both issues, we'd probably need to rework this part and make all
writes use the same approach. But for now let's revert this change to fix
the regression, that was also reported in [1].
I'll add a regression test for this problem separately.
[1] https://lists.apache.org/thread/56gnyc3tc0orjh5mfsqo9gpq1br59b01
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=1909088&r1=1909087&r2=1909088&view=diff
==============================================================================
--- apr/apr/trunk/CHANGES [utf-8] (original)
+++ apr/apr/trunk/CHANGES [utf-8] Wed Apr 12 12:51:03 2023
@@ -123,9 +123,6 @@ Changes for APR 2.0.0
*) apr_sockaddr_ip_getbuf, apr_sockaddr_ip_get: Append "%zone" for
IPv6 link-local addresses. [Joe Orton]
- *) 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=1909088&r1=1909087&r2=1909088&view=diff
==============================================================================
--- apr/apr/trunk/file_io/win32/open.c (original)
+++ apr/apr/trunk/file_io/win32/open.c Wed Apr 12 12:51:03 2023
@@ -369,6 +369,7 @@ 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=1909088&r1=1909087&r2=1909088&view=diff
==============================================================================
--- apr/apr/trunk/test/testfile.c (original)
+++ apr/apr/trunk/test/testfile.c Wed Apr 12 12:51:03 2023
@@ -1799,56 +1799,6 @@ 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);
-}
-
static void test_empty_read_buffered(abts_case *tc, void *data)
{
apr_status_t rv;
@@ -2303,7 +2253,6 @@ 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);
abts_run_test(suite, test_empty_read_buffered, NULL);
abts_run_test(suite, test_large_read_buffered, NULL);
abts_run_test(suite, test_two_large_reads_buffered, NULL);