svn commit: r1306076 - in /subversion/trunk/subversion: libsvn_subr/spillbuf.c tests/libsvn_subr/spillbuf-test.c

[gs...@apache.org]

Author: gstein
Date: Wed Mar 28 00:25:54 2012
New Revision: 1306076

URL: http://svn.apache.org/viewvc?rev=1306076&view=rev
Log:
While writing a test to validate spill buffer behavior when reading
precisely to the end... I discovered a bug. The SPILL_START value
within the spillbuf was not properly reset.

Fix the bug, and add the test which found it.

* subversion/libsvn_subr/spillbuf.c:
  (read_data): reset SPILL_START

* subversion/tests/libsvn_subr/spillbuf-test.c:
  (test_spillbuf_eof): add new test for validating behavior around the
    EOF of the spill file.

Modified:
    subversion/trunk/subversion/libsvn_subr/spillbuf.c
    subversion/trunk/subversion/tests/libsvn_subr/spillbuf-test.c

Modified: subversion/trunk/subversion/libsvn_subr/spillbuf.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/spillbuf.c?rev=1306076&r1=1306075&r2=1306076&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/spillbuf.c (original)
+++ subversion/trunk/subversion/libsvn_subr/spillbuf.c Wed Mar 28 00:25:54 2012
@@ -311,8 +311,10 @@ read_data(struct memblock_t **mem,
   /* Did we consume all the data from the spill file?  */
   if ((buf->spill_size -= (*mem)->size) == 0)
     {
+      /* Close and reset our spill file information.  */
       SVN_ERR(svn_io_file_close(buf->spill, scratch_pool));
       buf->spill = NULL;
+      buf->spill_start = 0;
     }
 
   /* *mem has been initialized. Done.  */

Modified: subversion/trunk/subversion/tests/libsvn_subr/spillbuf-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/spillbuf-test.c?rev=1306076&r1=1306075&r2=1306076&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/spillbuf-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/spillbuf-test.c Wed Mar 28 00:25:54 2012
@@ -361,6 +361,61 @@ test_spillbuf_rwfile(apr_pool_t *pool)
 }
 
 
+static svn_error_t *
+test_spillbuf_eof(apr_pool_t *pool)
+{
+  svn_spillbuf_t *buf = svn_spillbuf__create(4 /* blocksize */,
+                                             10 /* maxsize */,
+                                             pool);
+
+  SVN_ERR(svn_spillbuf__write(buf, "abcdef", 6, pool));
+  SVN_ERR(svn_spillbuf__write(buf, "ghijkl", 6, pool));
+  /* now: two blocks: 4 and 2 bytes, and 6 bytes in spill file.  */
+
+  CHECK_READ(buf, 12, "abcd", pool);
+  CHECK_READ(buf, 8, "ef", pool);
+  CHECK_READ(buf, 6, "ghij", pool);
+  CHECK_READ(buf, 2, "kl", pool);
+  /* The spill file should have been emptied and forgotten.  */
+
+  /* Assuming the spill file has been forgotten, this should result in
+     precisely the same behavior. Specifically: the initial write should
+     create two blocks, and the second write should be spilled. If there
+     *was* a spill file, then this written data would go into the file.  */
+  SVN_ERR(svn_spillbuf__write(buf, "abcdef", 6, pool));
+  SVN_ERR(svn_spillbuf__write(buf, "ghijkl", 6, pool));
+  CHECK_READ(buf, 12, "abcd", pool);
+  CHECK_READ(buf, 8, "ef", pool);
+  CHECK_READ(buf, 6, "ghij", pool);
+  CHECK_READ(buf, 2, "kl", pool);
+  /* The spill file should have been emptied and forgotten.  */
+
+  /* Now, let's do a sequence where we arrange to hit EOF precisely on
+     a block-sized read. Note: the second write must be more than 4 bytes,
+     or it will not cause a spill. We use 8 to get the right boundary.  */
+  SVN_ERR(svn_spillbuf__write(buf, "abcdef", 6, pool));
+  SVN_ERR(svn_spillbuf__write(buf, "ghijklmn", 8, pool));
+  CHECK_READ(buf, 14, "abcd", pool);
+  CHECK_READ(buf, 10, "ef", pool);
+  CHECK_READ(buf, 8, "ghij", pool);
+  CHECK_READ(buf, 4, "klmn", pool);
+  /* We discard the spill file when we know it has no data, rather than
+     upon hitting EOF (upon a read attempt). Thus, the spill file should
+     be gone.  */
+
+  /* Verify the forgotten spill file.  */
+  SVN_ERR(svn_spillbuf__write(buf, "abcdef", 6, pool));
+  SVN_ERR(svn_spillbuf__write(buf, "ghijkl", 6, pool));
+  CHECK_READ(buf, 12, "abcd", pool);
+  CHECK_READ(buf, 8, "ef", pool);
+  CHECK_READ(buf, 6, "ghij", pool);
+  /* Two unread bytes remaining in the spill file.  */
+  SVN_TEST_ASSERT(svn_spillbuf__get_size(buf) == 2);
+
+  return SVN_NO_ERROR;
+}
+
+
 /* The test table.  */
 struct svn_test_descriptor_t test_funcs[] =
   {
@@ -373,5 +428,6 @@ struct svn_test_descriptor_t test_funcs[
     SVN_TEST_PASS2(test_spillbuf_reader, "spill buffer reader test"),
     SVN_TEST_PASS2(test_spillbuf_stream, "spill buffer stream test"),
     SVN_TEST_PASS2(test_spillbuf_rwfile, "read/write spill file"),
+    SVN_TEST_PASS2(test_spillbuf_eof, "validate reaching EOF of spill file"),
     SVN_TEST_NULL
   };