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

Re: [PATCH] svn_spillbuf__is_empty()

From: Daniel Shahaf <danielsh_at_elego.de>
Date: Thu, 22 Sep 2011 06:22:11 +0300

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;
> }
> ]]]
Received on 2011-09-22 05:26:14 CEST

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