[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: Greg Stein <gstein_at_gmail.com>
Date: Thu, 22 Sep 2011 06:12:58 -0400

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

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