[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] svn_spillbuf__is_empty()

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Thu, 22 Sep 2011 05:53:03 +0300

[[[
Remove an edge case from svn_spillbuf__is_empty().

* subversion/include/private/svn_subr_private.h
  (svn_spillbuf_is_empty): Stop documenting this bug in the doc string.

* subversion/libsvn_subr/spillbuf.c
  (read_data): Also check for EOF in the non-error code path.

* subversion/tests/libsvn_subr/spillbuf-test.c
  (test_spillbuf_interleaving): Test svn_spillbuf_is_empty() a bit more.
     (The last of the new calls failed before this patch.)
]]]

[[[
Index: subversion/include/private/svn_subr_private.h
===================================================================
--- subversion/include/private/svn_subr_private.h (revision 1173935)
+++ subversion/include/private/svn_subr_private.h (working copy)
@@ -94,13 +94,7 @@ svn_spillbuf__create(apr_size_t blocksize,
                      apr_pool_t *result_pool);
 
 
-/* Determine whether the spill buffer has any content.
-
- Note: there is an edge case for a false positive. If the spill file was
- read *just* to the end of the file, but not past... then the spill
- buffer will not realize that no further content exists in the spill file.
- In this situation, svn_spillbuf_is_empty() will return TRUE, but an
- attempt to read content will detect that it has been exhausted. */
+/* Determine whether the spill buffer has any content. */
 svn_boolean_t
 svn_spillbuf__is_empty(const svn_spillbuf_t *buf);
 
Index: subversion/libsvn_subr/spillbuf.c
===================================================================
--- subversion/libsvn_subr/spillbuf.c (revision 1173932)
+++ subversion/libsvn_subr/spillbuf.c (working copy)
@@ -285,7 +285,8 @@ read_data(struct memblock_t **mem,
   /* Read some data from the spill file into the memblock. */
   err = svn_io_file_read(buf->spill, (*mem)->data, &(*mem)->size,
                          scratch_pool);
- if (err != NULL && APR_STATUS_IS_EOF(err->apr_err))
+ if ((err != NULL && APR_STATUS_IS_EOF(err->apr_err))
+ || apr_file_eof(buf->spill) == APR_EOF)
     {
       /* We've exhausted the file. Close it, so any new content will go
          into memory rather than the file. */
Index: subversion/tests/libsvn_subr/spillbuf-test.c
===================================================================
--- subversion/tests/libsvn_subr/spillbuf-test.c (revision 1173932)
+++ subversion/tests/libsvn_subr/spillbuf-test.c (working copy)
@@ -224,18 +224,22 @@ test_spillbuf_interleaving(apr_pool_t *pool)
   SVN_ERR(svn_spillbuf__write(buf, "GHIJKL", 6, pool));
   /* now: two blocks of 8 and 6 bytes, and 6 bytes spilled to a file */
 
+ SVN_TEST_ASSERT(! svn_spillbuf__is_empty(buf));
   SVN_ERR(svn_spillbuf__read(&readptr, &readlen, buf, pool));
   SVN_TEST_ASSERT(readptr != NULL
                   && readlen == 8
                   && memcmp(readptr, "qrstuvwx", 8) == 0);
+ SVN_TEST_ASSERT(! svn_spillbuf__is_empty(buf));
   SVN_ERR(svn_spillbuf__read(&readptr, &readlen, buf, pool));
   SVN_TEST_ASSERT(readptr != NULL
                   && readlen == 6
                   && memcmp(readptr, "ABCDEF", 6) == 0);
+ SVN_TEST_ASSERT(! svn_spillbuf__is_empty(buf));
   SVN_ERR(svn_spillbuf__read(&readptr, &readlen, buf, pool));
   SVN_TEST_ASSERT(readptr != NULL
                   && readlen == 6
                   && memcmp(readptr, "GHIJKL", 6) == 0);
+ SVN_TEST_ASSERT(svn_spillbuf__is_empty(buf));
 
   return SVN_NO_ERROR;
 }
]]]
Received on 2011-09-22 04:54:20 CEST

This is an archived mail posted to the Subversion Dev mailing list.