From 366c41be0cc7bafb70620371afca4d2cad7f74e0 Mon Sep 17 00:00:00 2001 From: Evgeny Kotkov Date: Wed, 12 Apr 2023 13:32:15 +0000 Subject: On 1.8.x branch: Merge r1909088, r1909089 from trunk: Revert r1808456 (Win32: Don't seek to the end when opening files with APR_FOPEN_APPEND). Add a regression test for the issue where appending to a buffered file was causing the content to be written at offset 0, rather than appended. git-svn-id: https://svn.apache.org/repos/asf/apr/apr/branches/1.8.x@1909093 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 -- file_io/win32/open.c | 1 + test/testfile.c | 96 ++++++++++++++++++++++++---------------------------- 3 files changed, 46 insertions(+), 54 deletions(-) diff --git a/CHANGES b/CHANGES index d402eb5c9..d5aa9a89c 100644 --- a/CHANGES +++ b/CHANGES @@ -22,9 +22,6 @@ Changes for APR 1.8.0 *) apr_file_gets: Optimize for buffered files on Windows. [Evgeny Kotkov ] - *) Don't seek to the end when opening files with APR_FOPEN_APPEND on Windows. - [Evgeny Kotkov ] - *) apr_file_write: Optimize large writes to buffered files on Windows. [Evgeny Kotkov ] diff --git a/file_io/win32/open.c b/file_io/win32/open.c index b6c01432e..e08f68e48 100644 --- a/file_io/win32/open.c +++ b/file_io/win32/open.c @@ -445,6 +445,7 @@ APR_DECLARE(apr_status_t) apr_file_open(apr_file_t **new, const char *fname, if (flag & APR_FOPEN_APPEND) { (*new)->append = 1; + SetFilePointer((*new)->filehand, 0, NULL, FILE_END); } if (flag & APR_FOPEN_BUFFERED) { (*new)->buffered = 1; diff --git a/test/testfile.c b/test/testfile.c index 660e3d27d..ca0a5a564 100644 --- a/test/testfile.c +++ b/test/testfile.c @@ -1893,56 +1893,6 @@ static void test_append_locked(abts_case *tc, void *data) 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; @@ -2301,6 +2251,50 @@ static void test_read_buffered_seek(abts_case *tc, void *data) apr_file_remove(fname, p); } +static void test_append_buffered(abts_case *tc, void *data) +{ + apr_status_t rv; + apr_file_t *f; + const char *fname = "data/testappend_buffered.dat"; + apr_size_t bytes_written; + 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_write_full(f, "abc", 3, &bytes_written); + APR_ASSERT_SUCCESS(tc, "write to file", rv); + apr_file_close(f); + + /* Re-open it with APR_FOPEN_APPEND and APR_FOPEN_BUFFERED. */ + rv = apr_file_open(&f, fname, + APR_FOPEN_READ | APR_FOPEN_WRITE | + APR_FOPEN_APPEND | APR_FOPEN_BUFFERED, + APR_FPROT_OS_DEFAULT, p); + APR_ASSERT_SUCCESS(tc, "open file", rv); + + /* Append to the file. */ + rv = apr_file_write_full(f, "def", 3, &bytes_written); + APR_ASSERT_SUCCESS(tc, "write to file", rv); + apr_file_close(f); + + rv = apr_file_open(&f, fname, APR_FOPEN_READ, + APR_FPROT_OS_DEFAULT, p); + APR_ASSERT_SUCCESS(tc, "open file", rv); + + /* Test the file contents. */ + rv = apr_file_gets(buf, sizeof(buf), f); + APR_ASSERT_SUCCESS(tc, "read from file", rv); + ABTS_STR_EQUAL(tc, "abcdef", buf); + + apr_file_close(f); + apr_file_remove(fname, p); +} + abts_suite *testfile(abts_suite *suite) { suite = ADD_SUITE(suite) @@ -2359,7 +2353,6 @@ abts_suite *testfile(abts_suite *suite) abts_run_test(suite, test_datasync_on_stream, 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); @@ -2367,6 +2360,7 @@ abts_suite *testfile(abts_suite *suite) abts_run_test(suite, test_read_buffered_spanning_over_bufsize, NULL); abts_run_test(suite, test_single_byte_reads_buffered, NULL); abts_run_test(suite, test_read_buffered_seek, NULL); + abts_run_test(suite, test_append_buffered, NULL); return suite; } -- cgit v1.2.1