On Wed, Sep 21, 2011 at 22:53, Daniel Shahaf <[email protected]> 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
