On 2/8/11 4:01 PM, stef...@apache.org wrote:
Author: stefan2
Date: Wed Feb 9 00:01:04 2011
New Revision: 1068695
URL: http://svn.apache.org/viewvc?rev=1068695&view=rev
Log:
Introduce svn_stream_skip() and svn_stream_buffered() stream API functions
and implement them for all stream types.
Hi Stefan,
I read through the public headers and it wasn't clear to me what skip did. I
assumed that the function takes the number of bytes to skip and seeks.
typedef svn_error_t *(*svn_skip_fn_t)(void *baton,
apr_size_t *count);
But in looking at the implementation, it actually consumes all the bytes. Maybe
it should be renamed to consume, because skip, to me, sounds like a cheap seek,
instead of a potentially more expensive read.
I suggest adding some more documentation here and/or renaming skip. Maybe
consume?
+
+/* Skip data from any stream by reading and simply discarding it. */
+static svn_error_t *
+skip_default_handler(void *baton, apr_size_t *count, svn_read_fn_t read_fn)
+{
+ apr_size_t total_bytes_read = 0;
+ apr_size_t bytes_read;
+ char buffer[4096];
+ svn_error_t *err = SVN_NO_ERROR;
+
+ while ((total_bytes_read< *count)&& !err)
+ {
+ bytes_read = sizeof(buffer)< *count ? sizeof(buffer) : *count;
Since *count isn't being decremented, then you could read past the *count bytes
if *count isn't evenly divisible by 4096.
Sounds like a unit test for skipping is needed.
static svn_error_t *
+skip_handler_apr(void *baton, apr_size_t *count)
+{
+ struct baton_apr *btn = baton;
+ apr_off_t offset = *count;
+
+ SVN_ERR(svn_io_file_seek(btn->file, SEEK_CUR,&offset, btn->pool));
APR_CUR
+ *count = offset;
skip_default_handler() returns the number of bytes actually read while
skip_handler_apr() returns the current offset. If you were already N bytes into
the stream, then the two functions don't return the same conceptual value.
+static svn_boolean_t
+buffered_handler_apr(void *baton)
+{
+ struct baton_apr *btn = baton;
+ return apr_file_buffer_size_get(btn->file) != 0;
We have systems with APR 1.2 and it doesn't provide apr_file_buffer_size_get().
Modified:
subversion/branches/integrate-stream-api-extensions/subversion/libsvn_subr/subst.c
URL:
http://svn.apache.org/viewvc/subversion/branches/integrate-stream-api-extensions/subversion/libsvn_subr/subst.c?rev=1068695&r1=1068694&r2=1068695&view=diff
==============================================================================
---
subversion/branches/integrate-stream-api-extensions/subversion/libsvn_subr/subst.c
(original)
+++
subversion/branches/integrate-stream-api-extensions/subversion/libsvn_subr/subst.c
Wed Feb 9 00:01:04 2011
@@ -1249,6 +1249,27 @@ translated_stream_read(void *baton,
return SVN_NO_ERROR;
}
+/* Implements svn_skip_fn_t. */
+static svn_error_t *
+translated_stream_skip(void *baton,
+ apr_size_t *count)
+{
+ apr_size_t total_bytes_read = 0;
+ apr_size_t bytes_read;
+ char buffer[SVN__STREAM_CHUNK_SIZE];
+ svn_error_t *err = SVN_NO_ERROR;
+
+ while ((total_bytes_read< *count)&& !err)
+ {
+ bytes_read = sizeof(buffer)< *count ? sizeof(buffer) : *count;
Same issue here with skipping.
Blair
