On Wed, Sep 21, 2011 at 22:53, Daniel Shahaf <danielsh_at_elego.de> wrote:
> [[[
> 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.
I don't think that apr_file_eof() can predictively determine EOF
unless/until you try and read off the end of the file. It is quite
conceivable to arrange a situation where we read precisely the number
of bytes in the file (ie. the remaining amount matches blocksize).
Looking apr_file_eof(), it would return that data without setting
thefile->eof_hit.
>...
> +++ 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 */
By writing "GHIJKLMN" here, that would drop 8 bytes into the spill 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));
... and I would guess this read would return 8 bytes and not be marked as empty.
Cheers,
-g
Received on 2011-09-22 12:13:30 CEST