You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by jo...@apache.org on 2005/09/04 14:21:49 UTC

svn commit: r278584 - in /apr/apr/branches/1.2.x: CHANGES file_io/unix/readwrite.c test/data/ test/testfile.c

Author: jorton
Date: Sun Sep  4 05:21:44 2005
New Revision: 278584

URL: http://svn.apache.org/viewcvs?rev=278584&view=rev
Log:
Merge r234013, r239221 from trunk:

* file_io/unix/readwrite.c (apr_file_write): Catch apr_file_flush()
failure for buffered files.

* file_io/unix/readwrite.c (apr_file_read, apr_file_gets): Handle the
apr_file_flush() return value when flushing buffered writes.

* test/testfile.c (test_fail_read_flush, test_fail_write_flush): Add
test cases.

Submitted by: Erik Huelsmann <ehuels gmail.com>, jorton

Modified:
    apr/apr/branches/1.2.x/CHANGES
    apr/apr/branches/1.2.x/file_io/unix/readwrite.c
    apr/apr/branches/1.2.x/test/data/   (props changed)
    apr/apr/branches/1.2.x/test/testfile.c

Modified: apr/apr/branches/1.2.x/CHANGES
URL: http://svn.apache.org/viewcvs/apr/apr/branches/1.2.x/CHANGES?rev=278584&r1=278583&r2=278584&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/CHANGES (original)
+++ apr/apr/branches/1.2.x/CHANGES Sun Sep  4 05:21:44 2005
@@ -1,5 +1,11 @@
 Changes for APR 1.2.2
 
+  *) Fix apr_file_gets() and apr_file_read() to catch write failures
+     when flushing pending writes for a buffered file.  [Joe Orton]
+
+  *) Fix apr_file_write() infinite loop on write failure for buffered
+     files.  [Erik Huelsmann <ehuels gmail.com>]
+
   *) Fix error handling where apr_uid_* and apr_gid_* could return
      APR_SUCCESS in failure cases.  PR 34053 continued.  [Joe Orton]
 

Modified: apr/apr/branches/1.2.x/file_io/unix/readwrite.c
URL: http://svn.apache.org/viewcvs/apr/apr/branches/1.2.x/file_io/unix/readwrite.c?rev=278584&r1=278583&r2=278584&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/file_io/unix/readwrite.c (original)
+++ apr/apr/branches/1.2.x/file_io/unix/readwrite.c Sun Sep  4 05:21:44 2005
@@ -47,7 +47,15 @@
 #endif
 
         if (thefile->direction == 1) {
-            apr_file_flush(thefile);
+            rv = apr_file_flush(thefile);
+            if (rv) {
+#if APR_HAS_THREADS
+                if (thefile->thlock) {
+                    apr_thread_mutex_unlock(thefile->thlock);
+                }
+#endif
+                return rv;
+            }
             thefile->bufpos = 0;
             thefile->direction = 0;
             thefile->dataRead = 0;
@@ -170,7 +178,7 @@
         rv = 0;
         while (rv == 0 && size > 0) {
             if (thefile->bufpos == APR_FILE_BUFSIZE)   /* write buffer is full*/
-                apr_file_flush(thefile);
+                rv = apr_file_flush(thefile);
 
             blocksize = size > APR_FILE_BUFSIZE - thefile->bufpos ? 
                         APR_FILE_BUFSIZE - thefile->bufpos : size;
@@ -333,7 +341,16 @@
 #endif
 
         if (thefile->direction == 1) {
-            apr_file_flush(thefile);
+            rv = apr_file_flush(thefile);
+            if (rv) {
+#if APR_HAS_THREADS
+                if (thefile->thlock) {
+                    apr_thread_mutex_unlock(thefile->thlock);
+                }
+#endif
+                return rv;
+            }
+
             thefile->direction = 0;
             thefile->bufpos = 0;
             thefile->dataRead = 0;

Propchange: apr/apr/branches/1.2.x/test/data/
------------------------------------------------------------------------------
--- svn:ignore (original)
+++ svn:ignore Sun Sep  4 05:21:44 2005
@@ -1,4 +1,2 @@
-testbigfprintf.dat
-testputs.txt
-testwritev.txt
-testwritev_full.txt
+test*.dat
+test*.txt

Modified: apr/apr/branches/1.2.x/test/testfile.c
URL: http://svn.apache.org/viewcvs/apr/apr/branches/1.2.x/test/testfile.c?rev=278584&r1=278583&r2=278584&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/test/testfile.c (original)
+++ apr/apr/branches/1.2.x/test/testfile.c Sun Sep  4 05:21:44 2005
@@ -703,6 +703,73 @@
     free(to_write);
 }
 
+static void test_fail_write_flush(abts_case *tc, void *data)
+{
+    apr_file_t *f;
+    const char *fname = "data/testflush.dat";
+    apr_status_t rv;
+    char buf[APR_BUFFERSIZE];
+    int n;
+
+    apr_file_remove(fname, p);
+
+    APR_ASSERT_SUCCESS(tc, "open test file",
+                       apr_file_open(&f, fname,
+                                     APR_CREATE|APR_READ|APR_BUFFERED,
+                                     APR_UREAD|APR_UWRITE, p));
+
+    memset(buf, 'A', sizeof buf);
+
+    /* Try three writes.  One of these should fail when it exceeds the
+     * internal buffer and actually tries to write to the file, which
+     * was opened read-only and hence should be unwritable. */
+    for (n = 0, rv = APR_SUCCESS; n < 4 && rv == APR_SUCCESS; n++) {
+        apr_size_t bytes = sizeof buf;
+        rv = apr_file_write(f, buf, &bytes);
+    }
+
+    ABTS_ASSERT(tc, "failed to write to read-only buffered fd",
+                rv != APR_SUCCESS);
+
+    apr_file_close(f);
+}
+
+static void test_fail_read_flush(abts_case *tc, void *data)
+{
+    apr_file_t *f;
+    const char *fname = "data/testflush.dat";
+    apr_status_t rv;
+    char buf[2];
+
+    apr_file_remove(fname, p);
+
+    APR_ASSERT_SUCCESS(tc, "open test file",
+                       apr_file_open(&f, fname,
+                                     APR_CREATE|APR_READ|APR_BUFFERED,
+                                     APR_UREAD|APR_UWRITE, p));
+
+    /* this write should be buffered. */
+    APR_ASSERT_SUCCESS(tc, "buffered write should succeed",
+                       apr_file_puts("hello", f));
+
+    /* Now, trying a read should fail since the write must be flushed,
+     * and should fail with something other than EOF since the file is
+     * opened read-only. */
+    rv = apr_file_read_full(f, buf, 2, NULL);
+
+    ABTS_ASSERT(tc, "read should flush buffered write and fail",
+                rv != APR_SUCCESS && rv != APR_EOF);
+
+    /* Likewise for gets */
+    rv = apr_file_gets(buf, 2, f);
+
+    ABTS_ASSERT(tc, "gets should flush buffered write and fail",
+                rv != APR_SUCCESS && rv != APR_EOF);
+
+    apr_file_close(f);
+}
+
+
 abts_suite *testfile(abts_suite *suite)
 {
     suite = ADD_SUITE(suite)
@@ -733,6 +800,8 @@
     abts_run_test(suite, test_mod_neg, NULL);
     abts_run_test(suite, test_truncate, NULL);
     abts_run_test(suite, test_bigfprintf, NULL);
+    abts_run_test(suite, test_fail_write_flush, NULL);
+    abts_run_test(suite, test_fail_read_flush, NULL);
 
     return suite;
 }