If this is applied then it should probably get another hunk:

[[[
Index: subversion/libsvn_subr/spillbuf.c
===================================================================
--- subversion/libsvn_subr/spillbuf.c   (revision 1173942)
+++ subversion/libsvn_subr/spillbuf.c   (working copy)
@@ -299,15 +299,6 @@ read_data(struct memblock_t **mem,
       return svn_error_trace(err);
     }
 
-  /* If we didn't read anything from the file, then avoid returning a
-     memblock (ie. just like running out of content).  */
-  if ((*mem)->size == 0)
-    {
-      return_buffer(buf, *mem);
-      *mem = NULL;
-      return SVN_NO_ERROR;
-    }
-
   /* Mark the data that we consumed from the spill file.  */
   buf->spill_start += (*mem)->size;
 
]]]

but I haven't tested that yet.

Daniel Shahaf wrote on Thu, Sep 22, 2011 at 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;
>  }
> ]]]

Reply via email to