Author: julianfoad Date: Thu Jun 2 11:29:30 2011 New Revision: 1130500 URL: http://svn.apache.org/viewvc?rev=1130500&view=rev Log: Simplify the semantics of svn_stream_skip() by not promising to report the actual length skipped if an error (including EOF) occurs. Thus eliminate a bug in reporting that length in skip_handler_apr().
See email by Stefan Fuhrmann on 2011-06-02, "svn commit: r1072519 - Add svn_stream_skip(), svn_stream_buffered(), svn_stream_supports_mark()", <http://svn.haxx.se/dev/archive-2011-06/0038.shtml>. * subversion/include/svn_io.h (svn_skip_fn_t, svn_stream_skip): Change the '*len' in/out parameter to 'len', input-only. * subversion/libsvn_subr/stream.c (svn_stream_skip, skip_handler_disown, skip_handler_md5, skip_handler_stringbuf, skip_handler_string): Same. (skip_default_handler, skip_handler_apr): Same, and thus simplify. (stream_readline_chunky): Adjust a call. * subversion/tests/libsvn_subr/stream-test.c (test_stream_seek_file, test_stream_seek_stringbuf, test_stream_seek_translated): Adjust calls. Modified: subversion/trunk/subversion/include/svn_io.h subversion/trunk/subversion/libsvn_subr/stream.c subversion/trunk/subversion/tests/libsvn_subr/stream-test.c Modified: subversion/trunk/subversion/include/svn_io.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=1130500&r1=1130499&r2=1130500&view=diff ============================================================================== --- subversion/trunk/subversion/include/svn_io.h (original) +++ subversion/trunk/subversion/include/svn_io.h Thu Jun 2 11:29:30 2011 @@ -768,7 +768,7 @@ typedef svn_error_t *(*svn_read_fn_t)(vo * @since New in 1.7. */ typedef svn_error_t *(*svn_skip_fn_t)(void *baton, - apr_size_t *len); + apr_size_t len); /** Write handler function for a generic stream. @see svn_stream_t. */ typedef svn_error_t *(*svn_write_fn_t)(void *baton, @@ -1084,8 +1084,7 @@ svn_stream_read(svn_stream_t *stream, /** * Skip @a len bytes from a generic @a stream. If the stream is exhausted - * before @a *len bytes have been read, return an error and set @a *len - * to the actual number of bytes skipped. + * before @a len bytes have been read, return an error. * * @note No assumption can be made on the semantics of this function * other than that the stream read pointer will be advanced by *len @@ -1097,7 +1096,7 @@ svn_stream_read(svn_stream_t *stream, */ svn_error_t * svn_stream_skip(svn_stream_t *stream, - apr_size_t *len); + apr_size_t len); /** Write to a generic stream. @see svn_stream_t. */ svn_error_t * Modified: subversion/trunk/subversion/libsvn_subr/stream.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/stream.c?rev=1130500&r1=1130499&r2=1130500&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_subr/stream.c (original) +++ subversion/trunk/subversion/libsvn_subr/stream.c Thu Jun 2 11:29:30 2011 @@ -60,7 +60,7 @@ struct svn_stream_t { /*** Forward declarations. ***/ static svn_error_t * -skip_default_handler(void *baton, apr_size_t *count, svn_read_fn_t read_fn); +skip_default_handler(void *baton, apr_size_t len, svn_read_fn_t read_fn); /*** Generic streams. ***/ @@ -142,7 +142,7 @@ svn_stream_read(svn_stream_t *stream, ch svn_error_t * -svn_stream_skip(svn_stream_t *stream, apr_size_t *len) +svn_stream_skip(svn_stream_t *stream, apr_size_t len) { if (stream->skip_fn == NULL) return skip_default_handler(stream->baton, len, stream->read_fn); @@ -471,7 +471,7 @@ stream_readline_chunky(svn_stringbuf_t * /* Move the stream read pointer to the first position behind the EOL. */ SVN_ERR(svn_stream_seek(stream, mark)); - return svn_stream_skip(stream, &total_parsed); + return svn_stream_skip(stream, total_parsed); } /* Guts of svn_stream_readline() and svn_stream_readline_detect_eol(). @@ -626,24 +626,20 @@ svn_stream_contents_same2(svn_boolean_t /* 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) +skip_default_handler(void *baton, apr_size_t len, svn_read_fn_t read_fn) { - apr_size_t total_bytes_read = 0; apr_size_t bytes_read = 1; char buffer[4096]; - svn_error_t *err = SVN_NO_ERROR; - apr_size_t to_read = *count; + apr_size_t to_read = len; - while ((to_read > 0) && !err && (bytes_read > 0)) + while ((to_read > 0) && (bytes_read > 0)) { bytes_read = sizeof(buffer) < to_read ? sizeof(buffer) : to_read; - err = read_fn(baton, buffer, &bytes_read); - total_bytes_read += bytes_read; + SVN_ERR(read_fn(baton, buffer, &bytes_read)); to_read -= bytes_read; } - *count = total_bytes_read; - return err; + return SVN_NO_ERROR; } @@ -765,9 +761,9 @@ read_handler_disown(void *baton, char *b } static svn_error_t * -skip_handler_disown(void *baton, apr_size_t *count) +skip_handler_disown(void *baton, apr_size_t len) { - return svn_stream_skip(baton, count); + return svn_stream_skip(baton, len); } static svn_error_t * @@ -850,28 +846,12 @@ read_handler_apr(void *baton, char *buff } static svn_error_t * -skip_handler_apr(void *baton, apr_size_t *count) +skip_handler_apr(void *baton, apr_size_t len) { struct baton_apr *btn = baton; - apr_off_t offset = *count; - apr_off_t new_pos = *count; - apr_off_t current = 0; - svn_error_t *err; - - /* so far, we have not moved anything */ - *count = 0; + apr_off_t offset = len; - SVN_ERR(svn_io_file_seek(btn->file, APR_CUR, ¤t, btn->pool)); - err = svn_io_file_seek(btn->file, APR_CUR, &new_pos, btn->pool); - - /* Irrespective of errors, return the number of bytes we actually moved. - * If no new position has been returned from seek(), assume that no move - * happend and keep the *count==0 set earlier. - */ - if ((offset != new_pos) || (current == 0)) - *count = (apr_size_t)(new_pos - current); - - return err; + return svn_io_file_seek(btn->file, APR_CUR, &offset, btn->pool); } static svn_error_t * @@ -1439,10 +1419,10 @@ read_handler_md5(void *baton, char *buff } static svn_error_t * -skip_handler_md5(void *baton, apr_size_t *count) +skip_handler_md5(void *baton, apr_size_t len) { struct md5_stream_baton *btn = baton; - return svn_stream_skip(btn->proxy, count); + return svn_stream_skip(btn->proxy, len); } static svn_error_t * @@ -1539,13 +1519,13 @@ read_handler_stringbuf(void *baton, char } static svn_error_t * -skip_handler_stringbuf(void *baton, apr_size_t *count) +skip_handler_stringbuf(void *baton, apr_size_t len) { struct stringbuf_stream_baton *btn = baton; apr_size_t left_to_read = btn->str->len - btn->amt_read; - *count = (*count > left_to_read) ? left_to_read : *count; - btn->amt_read += *count; + len = (len > left_to_read) ? left_to_read : len; + btn->amt_read += len; return SVN_NO_ERROR; } @@ -1675,13 +1655,13 @@ seek_handler_string(void *baton, const s } static svn_error_t * -skip_handler_string(void *baton, apr_size_t *count) +skip_handler_string(void *baton, apr_size_t len) { struct string_stream_baton *btn = baton; apr_size_t left_to_read = btn->str->len - btn->amt_read; - *count = (*count > left_to_read) ? left_to_read : *count; - btn->amt_read += *count; + len = (len > left_to_read) ? left_to_read : len; + btn->amt_read += len; return SVN_NO_ERROR; } Modified: subversion/trunk/subversion/tests/libsvn_subr/stream-test.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/stream-test.c?rev=1130500&r1=1130499&r2=1130500&view=diff ============================================================================== --- subversion/trunk/subversion/tests/libsvn_subr/stream-test.c (original) +++ subversion/trunk/subversion/tests/libsvn_subr/stream-test.c Thu Jun 2 11:29:30 2011 @@ -262,7 +262,6 @@ test_stream_seek_file(apr_pool_t *pool) apr_status_t status; static const char *NL = APR_EOL_STR; svn_stream_mark_t *mark; - apr_size_t count; status = apr_file_open(&f, fname, (APR_READ | APR_WRITE | APR_CREATE | APR_TRUNCATE | APR_DELONCLOSE), APR_OS_DEFAULT, pool); @@ -305,12 +304,10 @@ test_stream_seek_file(apr_pool_t *pool) SVN_ERR(svn_stream_readline(stream, &line, NL, &eof, pool)); SVN_TEST_ASSERT(eof); - /* Go back to the begin of last line and try to skip it + /* Go back to the beginning of the last line and try to skip it * NOT including the EOL. */ SVN_ERR(svn_stream_seek(stream, mark)); - count = strlen(file_data[1]); - SVN_ERR(svn_stream_skip(stream, &count)); - SVN_TEST_ASSERT(count == strlen(file_data[1])); + SVN_ERR(svn_stream_skip(stream, strlen(file_data[1]))); /* The remaining line should be empty */ SVN_ERR(svn_stream_readline(stream, &line, NL, &eof, pool)); SVN_TEST_ASSERT(! eof && strcmp(line->data, "") == 0); @@ -351,9 +348,7 @@ test_stream_seek_stringbuf(apr_pool_t *p /* Go back to the begin of last word and try to skip some of it */ SVN_ERR(svn_stream_seek(stream, mark)); - len = 2; - SVN_ERR(svn_stream_skip(stream, &len)); - SVN_TEST_ASSERT(len == 2); + SVN_ERR(svn_stream_skip(stream, 2)); /* The remaining line should be empty */ len = 3; SVN_ERR(svn_stream_read(stream, buf, &len)); @@ -400,9 +395,7 @@ test_stream_seek_translated(apr_pool_t * SVN_TEST_STRING_ASSERT(buf, " was"); SVN_ERR(svn_stream_seek(translated_stream, mark)); - len = 2; - SVN_ERR(svn_stream_skip(translated_stream, &len)); - SVN_TEST_ASSERT(len == 2); + SVN_ERR(svn_stream_skip(translated_stream, 2)); len = 2; SVN_ERR(svn_stream_read(translated_stream, buf, &len)); SVN_TEST_ASSERT(len == 2); @@ -424,9 +417,7 @@ test_stream_seek_translated(apr_pool_t * SVN_TEST_STRING_ASSERT(buf, " expanded"); SVN_ERR(svn_stream_seek(translated_stream, mark)); - len = 6; - SVN_ERR(svn_stream_skip(translated_stream, &len)); - SVN_TEST_ASSERT(len == 6); + SVN_ERR(svn_stream_skip(translated_stream, 6)); len = 3; SVN_ERR(svn_stream_read(translated_stream, buf, &len)); SVN_TEST_ASSERT(len == 3); @@ -448,9 +439,7 @@ test_stream_seek_translated(apr_pool_t * SVN_TEST_STRING_ASSERT(buf, " $Tw"); SVN_ERR(svn_stream_seek(translated_stream, mark)); - len = 2; - SVN_ERR(svn_stream_skip(translated_stream, &len)); - SVN_TEST_ASSERT(len == 2); + SVN_ERR(svn_stream_skip(translated_stream, 2)); len = 2; SVN_ERR(svn_stream_read(translated_stream, buf, &len)); SVN_TEST_ASSERT(len == 2); @@ -472,9 +461,7 @@ test_stream_seek_translated(apr_pool_t * SVN_TEST_STRING_ASSERT(buf, "o"); SVN_ERR(svn_stream_seek(translated_stream, mark)); - len = 2; - SVN_ERR(svn_stream_skip(translated_stream, &len)); - SVN_TEST_ASSERT(len == 1); + SVN_ERR(svn_stream_skip(translated_stream, 2)); len = 1; SVN_ERR(svn_stream_read(translated_stream, buf, &len)); SVN_TEST_ASSERT(len == 0);
