Author: stsp Date: Thu Jul 8 22:42:46 2010 New Revision: 961969 URL: http://svn.apache.org/viewvc?rev=961969&view=rev Log: Fix issue #3555, 'Remove the "line_filter" and "line_transformer" callbacks from svn_stream_t'.
Add custom functions similar to svn_stream_readline() to the diff parsing API, and remove the line-filter and transformer callbacks from the generic svn_stream_readline(). This change is mostly mechanical and doesn't change any functionality. See issue #3555 for rationale, and also this thread for related discussion: Date: Wed, 7 Jul 2010 20:39:02 +0200 From: Stefan Sperling To: d...@subversion.apache.org Subject: [PATCH] issue #3555: make svn_stream_readline() a stream method Message-ID: <20100707183902.gk5...@ted.stsp.name> http://svn.haxx.se/dev/archive-2010-07/0176.shtml * subversion/include/svn_diff.h (svn_hunk_t): Adjust documentation of the diff_text, original_text, and modified_text streams. Add new field 'reverse'. (svn_diff_hunk_readline_original_text, svn_diff_hunk_readline_modified_text, svn_diff_hunk_readline_diff_text): Declare. * subversion/include/svn_io.h (svn_io_line_filter_cb_t, svn_io_line_transformer_cb_t, svn_stream_set_line_filter_callback, svn_stream_set_line_transformer_callback): Remove. (svn_stream_readline): Remove related documentation. * subversion/libsvn_diff/parse-diff.c (original_line_filter, modified_line_filter, remove_leading_char_transformer, reverse_diff_transformer): Remove. (scan_eol): New, copied from libsvn_subr/stream.c. (readline): New helper function for reading hunk text streams. (svn_diff_hunk_readline_original_text, svn_diff_hunk_readline_modified_text, svn_diff_hunk_readline_diff_text): New public interfaces for reading texts from hunks, implemented as wrappers around the readline() helper method. These must be used instead of the standard stream readline functions. We should consider making svn_hunk_t an opaque type to properly prevent access to the raw text streams. (parse_next_hunk): Remove code setting line_filter and line_transformer callbacks on streams. Initialise hunk->reverse. * subversion/libsvn_subr/stream.c (svn_stream_t, svn_stream_create): Remove line_filter_cb and line_transformer_cb. (svn_stream_set_line_filter_callback, svn_stream_set_line_transformer_callback, line_filter, line_transformer): Remove. (svn_stream_readline): Remove handling of line_filter and line_transformer callbacks. * subversion/libsvn_client/patch.c (match_hunk, reject_hunk, apply_hunk): Use the new svn_diff_hunk_readline_* functions to read texts from hunks. * subversion/tests/libsvn_diff/parse-diff-test.c (check_content): Also use new svn_diff_hunk_readline_* functions. Tweak the parameter list and rename some variables accordingly. (test_parse_unidiff, test_parse_git_diff, test_parse_git_tree_and_text_diff, test_bad_git_diff_headers, test_parse_property_diff, test_parse_property_and_text_diff): Track changes made to check_content. * subversion/tests/libsvn_subr/stream-test.c (line_filter, test_stream_line_filter, line_transformer, test_stream_line_transformer, test_stream_line_filter_and_transformer): Remove these tests. (test_funcs): Remove removed tests. Modified: subversion/trunk/subversion/include/svn_diff.h subversion/trunk/subversion/include/svn_io.h subversion/trunk/subversion/libsvn_client/patch.c subversion/trunk/subversion/libsvn_diff/parse-diff.c subversion/trunk/subversion/libsvn_subr/stream.c subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c subversion/trunk/subversion/tests/libsvn_subr/stream-test.c Modified: subversion/trunk/subversion/include/svn_diff.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_diff.h?rev=961969&r1=961968&r2=961969&view=diff ============================================================================== --- subversion/trunk/subversion/include/svn_diff.h (original) +++ subversion/trunk/subversion/include/svn_diff.h Thu Jul 8 22:42:46 2010 @@ -790,11 +790,22 @@ typedef enum svn_diff_operation_kind_e * * @since New in 1.7. */ typedef struct svn_hunk_t { - /** The hunk's unidiff text as it appeared in the patch file, - * without range information. */ + /** + * The hunk's unidiff text as it appeared in the patch file, + * without range information. + * This stream should not be read from directly, because it will + * return the wrong result for reversed hunks. + * @see svn_diff_hunk_readline_diff_text() + * However, the stream supports resetting and it is safe to reset it. + */ svn_stream_t *diff_text; /** + * Whether the hunk is being interpreted in reverse. + */ + svn_boolean_t reverse; + + /** * The original and modified texts in the hunk range. * Derived from the diff text. * @@ -821,11 +832,14 @@ typedef struct svn_hunk_t { * printf("I like Subversion!\n"); * } * - * Because these streams make use of line filtering and transformation, - * they should only be read line-by-line with svn_stream_readline(). - * Reading them with svn_stream_read() will not yield the expected result, - * because it will return the unidiff text from the patch file unmodified. - * The streams support resetting. + * These streams should not be read from directly. + * Reading them with svn_stream_read() or svn_stream_readline() will + * not yield the expected result, because that will return the unidiff + * text from the patch file unmodified. + * @see svn_diff_hunk_readline_original_text() + * @see svn_diff_hunk_readline_modified_text() + * + * However, the streams support resetting and it is safe to reset them. */ svn_stream_t *original_text; svn_stream_t *modified_text; @@ -897,6 +911,64 @@ svn_diff_parse_next_patch(svn_patch_t ** apr_pool_t *scratch_pool); /** + * Allocate @a *stringbuf in @a result_pool, and read into it one line + * of the original text of @a hunk. + * The line-terminator is detected automatically and stored in @a *eol + * if @a eol is not NULL. + * If EOF is reached and the stream does not end with a newline character, + * and @a eol is not NULL, @a *eol is set to NULL. + * Temporary allocations will be performed in @a scratch_pool. + * + * @see svn_hunk_t + * @since New in 1.7. + */ +svn_error_t * +svn_diff_hunk_readline_original_text(const svn_hunk_t *hunk, + svn_stringbuf_t **stringbuf, + const char **eol, + svn_boolean_t *eof, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool); + +/** + * Like svn_diff_hunk_readline_original_text(), but it returns lines from + * the modified text of the hunk. + * + * @see svn_hunk_t + * @since New in 1.7. + */ +svn_error_t * +svn_diff_hunk_readline_modified_text(const svn_hunk_t *hunk, + svn_stringbuf_t **stringbuf, + const char **eol, + svn_boolean_t *eof, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool); + +/** + * Allocate @a *stringbuf in @a result_pool, and read into it one line + * of the diff text of @a hunk. The first line returned is the hunk header. + * Any subsequent lines are unidiff data (starting with '+', '-', or ' '). + * If the @a hunk is being interpreted in reverse (i.e. the reverse + * parameter of svn_diff_parse_next_patch() was @c TRUE), the diff + * text will be returned in reversed form. + * The line-terminator is detected automatically and stored in @a *eol + * if @a eol is not NULL. + * If EOF is reached and the stream does not end with a newline character, + * and @a eol is not NULL, @a *eol is set to NULL. + * Temporary allocations will be performed in @a scratch_pool. + * + * @since New in 1.7. + */ +svn_error_t * +svn_diff_hunk_readline_diff_text(const svn_hunk_t *hunk, + svn_stringbuf_t **stringbuf, + const char **eol, + svn_boolean_t *eof, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool); + +/** * Dispose of @a patch, closing any streams used by it. * * @since New in 1.7. Modified: subversion/trunk/subversion/include/svn_io.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=961969&r1=961968&r2=961969&view=diff ============================================================================== --- subversion/trunk/subversion/include/svn_io.h (original) +++ subversion/trunk/subversion/include/svn_io.h Thu Jul 8 22:42:46 2010 @@ -780,41 +780,6 @@ typedef svn_error_t *(*svn_io_mark_fn_t) typedef svn_error_t *(*svn_io_seek_fn_t)(void *baton, svn_stream_mark_t *mark); -/** Line-filtering callback function for a generic stream. - * @a baton is the stream's baton. - * @see svn_stream_t, svn_stream_set_baton() and svn_stream_readline(). - * - * @since New in 1.7. - */ -typedef svn_error_t *(*svn_io_line_filter_cb_t)(svn_boolean_t *filtered, - const char *line, - void *baton, - apr_pool_t *scratch_pool); - -/** A callback function, invoked by svn_stream_readline(), which can perform - * arbitary transformations on the line before it is passed back to the caller - * of svn_stream_readline(). - * - * Returns a transformed stringbuf in @a buf, allocated in @a result_pool. - * This callback gets invoked on lines which were not filtered by the - * line-filtering callback function. - * - * Implementations should always at least return an empty stringbuf. - * It is a fatal error if an implementation returns @a *buf as NULL. - * - * @a baton is the stream's baton. - * - * @see svn_stream_t, svn_stream_set_baton(), svn_io_line_filter_cb_t and - * svn_stream_readline(). - * - * @since New in 1.7. - */ -typedef svn_error_t *(*svn_io_line_transformer_cb_t)(svn_stringbuf_t **buf, - const char *line, - void *baton, - apr_pool_t *result_pool, - apr_pool_t *scratch_pool); - /** Create a generic stream. @see svn_stream_t. */ svn_stream_t * svn_stream_create(void *baton, @@ -864,24 +829,6 @@ void svn_stream_set_seek(svn_stream_t *stream, svn_io_seek_fn_t seek_fn); -/** Set @a stream's line-filtering callback function to @a line_filter_cb - * - * @since New in 1.7. - */ -void -svn_stream_set_line_filter_callback(svn_stream_t *stream, - svn_io_line_filter_cb_t line_filter_cb); - -/** Set @a streams's line-transforming callback function to - * @a line_transformer_cb. - * - * @since New in 1.7. - */ -void -svn_stream_set_line_transformer_callback( - svn_stream_t *stream, - svn_io_line_transformer_cb_t line_transformer_cb); - /** Create a stream that is empty for reading and infinite for writing. */ svn_stream_t * svn_stream_empty(apr_pool_t *pool); @@ -1203,19 +1150,6 @@ svn_stream_printf_from_utf8(svn_stream_t * * If @a stream runs out of bytes before encountering a line-terminator, * then set @a *eof to @c TRUE, otherwise set @a *eof to FALSE. - * - * If a line-filter callback function was set on the stream using - * svn_stream_set_line_filter_callback(), lines will only be returned - * if they pass the filtering decision of the callback function. - * If end-of-file is encountered while reading the line and the line - * is filtered, @a *stringbuf will be empty. - * - * If a line-transformer callback function was set on the stream using - * svn_stream_set_line_transformer_callback(), lines will be returned - * transformed, in a way determined by the callback. - * - * Note that the line-transformer callback gets called after the line-filter - * callback, not before. */ svn_error_t * svn_stream_readline(svn_stream_t *stream, Modified: subversion/trunk/subversion/libsvn_client/patch.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=961969&r1=961968&r2=961969&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_client/patch.c (original) +++ subversion/trunk/subversion/libsvn_client/patch.c Thu Jul 8 22:42:46 2010 @@ -630,9 +630,10 @@ match_hunk(svn_boolean_t *matched, patch svn_pool_clear(iterpool); - SVN_ERR(svn_stream_readline_detect_eol(hunk->original_text, - &hunk_line, NULL, - &hunk_eof, iterpool)); + SVN_ERR(svn_diff_hunk_readline_original_text(hunk, &hunk_line, + NULL, &hunk_eof, + iterpool, iterpool)); + /* Contract keywords, if any, before matching. */ SVN_ERR(svn_subst_translate_cstring2(hunk_line->data, &hunk_line_translated, @@ -675,8 +676,9 @@ match_hunk(svn_boolean_t *matched, patch { /* If the target has no newline at end-of-file, we get an EOF * indication for the target earlier than we do get it for the hunk. */ - SVN_ERR(svn_stream_readline_detect_eol(hunk->original_text, &hunk_line, - NULL, &hunk_eof, iterpool)); + SVN_ERR(svn_diff_hunk_readline_original_text(hunk, &hunk_line, + NULL, &hunk_eof, + iterpool, iterpool)); if (hunk_line->len == 0 && hunk_eof) *matched = lines_matched; else @@ -927,8 +929,8 @@ reject_hunk(patch_target_t *target, hunk svn_pool_clear(iterpool); - SVN_ERR(svn_stream_readline_detect_eol(hi->hunk->diff_text, &hunk_line, - &eol_str, &eof, iterpool)); + SVN_ERR(svn_diff_hunk_readline_diff_text(hi->hunk, &hunk_line, &eol_str, + &eof, iterpool, iterpool)); if (! eof) { if (hunk_line->len >= 1) @@ -992,9 +994,9 @@ apply_hunk(patch_target_t *target, hunk_ svn_pool_clear(iterpool); - SVN_ERR(svn_stream_readline_detect_eol(hi->hunk->modified_text, - &hunk_line, &eol_str, - &eof, iterpool)); + SVN_ERR(svn_diff_hunk_readline_modified_text(hi->hunk, &hunk_line, + &eol_str, &eof, + iterpool, iterpool)); lines_read++; if (! eof && lines_read > hi->fuzz && lines_read <= hi->hunk->modified_length - hi->fuzz) Modified: subversion/trunk/subversion/libsvn_diff/parse-diff.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/parse-diff.c?rev=961969&r1=961968&r2=961969&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_diff/parse-diff.c (original) +++ subversion/trunk/subversion/libsvn_diff/parse-diff.c Thu Jul 8 22:42:46 2010 @@ -33,6 +33,8 @@ #include "svn_dirent_uri.h" #include "svn_diff.h" +#include "private/svn_eol_private.h" + /* Helper macro for readability */ #define starts_with(str, start) \ (strncmp((str), (start), strlen(start)) == 0) @@ -184,85 +186,229 @@ parse_hunk_header(const char *header, sv return TRUE; } -/* A stream line-filter which allows only original text from a hunk, - * and filters special lines (which start with a backslash). */ +/* Set *EOL to the first end-of-line string found in the stream + * accessed through READ_FN, MARK_FN and SEEK_FN, whose stream baton + * is BATON. Leave the stream read position unchanged. + * Allocate *EOL statically; POOL is a scratch pool. */ static svn_error_t * -original_line_filter(svn_boolean_t *filtered, const char *line, void *baton, - apr_pool_t *scratch_pool) +scan_eol(const char **eol, svn_stream_t *stream, apr_pool_t *pool) { - *filtered = (line[0] == '+' || line[0] == '\\'); - return SVN_NO_ERROR; -} + const char *eol_str; + svn_stream_mark_t *mark; -/* A stream line-filter which allows only modified text from a hunk, - * and filters special lines (which start with a backslash). */ -static svn_error_t * -modified_line_filter(svn_boolean_t *filtered, const char *line, void *baton, - apr_pool_t *scratch_pool) -{ - *filtered = (line[0] == '-' || line[0] == '\\'); - return SVN_NO_ERROR; -} + SVN_ERR(svn_stream_mark(stream, &mark, pool)); -/** line-transformer callback to shave leading diff symbols. */ -static svn_error_t * -remove_leading_char_transformer(svn_stringbuf_t **buf, - const char *line, - void *baton, - apr_pool_t *result_pool, - apr_pool_t *scratch_pool) -{ - if (line[0] == '+' || line[0] == '-' || line[0] == ' ') - *buf = svn_stringbuf_create(line + 1, result_pool); - else - *buf = svn_stringbuf_create(line, result_pool); + eol_str = NULL; + while (! eol_str) + { + char buf[512]; + apr_size_t len; + + len = sizeof(buf); + SVN_ERR(svn_stream_read(stream, buf, &len)); + if (len == 0) + break; /* EOF */ + eol_str = svn_eol__detect_eol(buf, buf + len); + } + + SVN_ERR(svn_stream_seek(stream, mark)); + + *eol = eol_str; return SVN_NO_ERROR; } -/** line-transformer callback to reverse a diff text. */ +/* A helper function similar to svn_stream_readline(), suitable for reading + * lines from a STREAM which has been mapped onto a hunk region within a + * unidiff patch file. + * + * Allocate *STRINGBUF in RESULT_POOL, and read into it one line from STREAM. + * + * The line-terminator is detected automatically and stored in *EOL + * if EOL is not NULL. If EOF is reached and the stream does not end + * with a newline character, and EOL is not NULL, *EOL is set to NULL. + * + * STREAM is expected to contain unidiff text. + * If PLAIN_DIFF is TRUE, return unidiff text read from STREAM unmodified. + * If PLAIN_DIFF is FALSE, shave off leading unidiff symbols ('+', '-', + * and ' ') from the line, and discard any lines commencing with the + * VERBOTEN character. VERBOTEN should be '+' or '-', and is ignored if + * PLAIN_DIFF is TRUE. + * + * SCRATCH_POOL is used for temporary allocations. + */ static svn_error_t * -reverse_diff_transformer(svn_stringbuf_t **buf, - const char *line, - void *baton, - apr_pool_t *result_pool, - apr_pool_t *scratch_pool) +readline(svn_stream_t *stream, + svn_stringbuf_t **stringbuf, + const char **eol, + svn_boolean_t *eof, + char verboten, + svn_boolean_t plain_diff, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) { - svn_hunk_t hunk; + svn_stringbuf_t *str; + apr_pool_t *iterpool; + svn_boolean_t filtered; + const char *eol_str; + + *eof = FALSE; + + iterpool = svn_pool_create(scratch_pool); + do + { + apr_size_t numbytes; + const char *match; + char c; + + svn_pool_clear(iterpool); + + /* Since we're reading one character at a time, let's at least + optimize for the 90% case. 90% of the time, we can avoid the + stringbuf ever having to realloc() itself if we start it out at + 80 chars. */ + str = svn_stringbuf_create_ensure(80, iterpool); + + SVN_ERR(scan_eol(&eol_str, stream, iterpool)); + if (eol) + *eol = eol_str; + if (eol_str == NULL) + { + /* No newline until EOF, EOL_STR can be anything. */ + eol_str = APR_EOL_STR; + } + + /* Read into STR up to and including the next EOL sequence. */ + match = eol_str; + numbytes = 1; + while (*match) + { + SVN_ERR(svn_stream_read(stream, &c, &numbytes)); + if (numbytes != 1) + { + /* a 'short' read means the stream has run out. */ + *eof = TRUE; + /* We know we don't have a whole EOL sequence, but ensure we + * don't chop off any partial EOL sequence that we may have. */ + match = eol_str; + /* Process this short (or empty) line just like any other + * except with *EOF set. */ + break; + } - /* ### Pass the already parsed hunk via the baton? - * ### Maybe we should really make svn_stream_readline() a proper stream - * ### method and override it instead of adding special callbacks? */ - if (parse_hunk_header(line, &hunk, "@@", FALSE, scratch_pool)) + if (c == *match) + match++; + else + match = eol_str; + + svn_stringbuf_appendbytes(str, &c, 1); + } + + svn_stringbuf_chop(str, match - eol_str); + filtered = (plain_diff == FALSE && + (str->data[0] == verboten || str->data[0] == '\\')); + } + while (filtered && ! *eof); + /* Not destroying the iterpool just yet since we still need STR + * which is allocated in it. */ + + if (filtered) { - *buf = svn_stringbuf_createf(result_pool, - "@@ -%lu,%lu +%lu,%lu @@", - hunk.modified_start, - hunk.modified_length, - hunk.original_start, - hunk.original_length); + /* EOF, return an empty string. */ + *stringbuf = svn_stringbuf_create_ensure(0, result_pool); } - else if (parse_hunk_header(line, &hunk, "##", FALSE, scratch_pool)) + else if (! plain_diff && + (str->data[0] == '+' || str->data[0] == '-' || str->data[0] == ' ')) { - *buf = svn_stringbuf_createf(result_pool, - "## -%lu,%lu +%lu,%lu ##", - hunk.modified_start, - hunk.modified_length, - hunk.original_start, - hunk.original_length); + /* Shave off leading unidiff symbols. */ + *stringbuf = svn_stringbuf_create(str->data + 1, result_pool); } - else if (line[0] == '+') + else { - *buf = svn_stringbuf_create(line, result_pool); - (*buf)->data[0] = '-'; + /* Return the line as-is. */ + *stringbuf = svn_stringbuf_dup(str, result_pool); } - else if (line[0] == '-') + + /* Done. RIP iterpool. */ + svn_pool_destroy(iterpool); + + return SVN_NO_ERROR; +} + +svn_error_t * +svn_diff_hunk_readline_original_text(const svn_hunk_t *hunk, + svn_stringbuf_t **stringbuf, + const char **eol, + svn_boolean_t *eof, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) +{ + return svn_error_return(readline(hunk->original_text, stringbuf, eol, eof, + hunk->reverse ? '-' : '+', + FALSE, result_pool, scratch_pool)); +} + +svn_error_t * +svn_diff_hunk_readline_modified_text(const svn_hunk_t *hunk, + svn_stringbuf_t **stringbuf, + const char **eol, + svn_boolean_t *eof, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) +{ + return svn_error_return(readline(hunk->modified_text, stringbuf, eol, eof, + hunk->reverse ? '+' : '-', + FALSE, result_pool, scratch_pool)); +} + +svn_error_t * +svn_diff_hunk_readline_diff_text(const svn_hunk_t *hunk, + svn_stringbuf_t **stringbuf, + const char **eol, + svn_boolean_t *eof, + apr_pool_t *result_pool, + apr_pool_t *scratch_pool) +{ + svn_hunk_t dummy; + svn_stringbuf_t *line; + + SVN_ERR(readline(hunk->diff_text, &line, eol, eof, '\0', TRUE, + result_pool, scratch_pool)); + + if (hunk->reverse) { - *buf = svn_stringbuf_create(line, result_pool); - (*buf)->data[0] = '+'; + if (parse_hunk_header(line->data, &dummy, "@@", FALSE, scratch_pool)) + { + /* Line is a hunk header, reverse it. */ + *stringbuf = svn_stringbuf_createf(result_pool, + "@@ -%lu,%lu +%lu,%lu @@", + hunk->modified_start, + hunk->modified_length, + hunk->original_start, + hunk->original_length); + } + else if (parse_hunk_header(line->data, &dummy, "##", FALSE, scratch_pool)) + { + /* Line is a hunk header, reverse it. */ + *stringbuf = svn_stringbuf_createf(result_pool, + "## -%lu,%lu +%lu,%lu ##", + hunk->modified_start, + hunk->modified_length, + hunk->original_start, + hunk->original_length); + } + else + { + if (line->data[0] == '+') + line->data[0] = '-'; + else if (line->data[0] == '-') + line->data[0] = '+'; + + *stringbuf = line; + } } else - *buf = svn_stringbuf_create(line, result_pool); + *stringbuf = line; return SVN_NO_ERROR; } @@ -515,9 +661,6 @@ parse_next_hunk(svn_hunk_t **hunk, original_text = svn_stream_from_aprfile_range_readonly(f, FALSE, start, end, result_pool); - svn_stream_set_line_filter_callback(original_text, original_line_filter); - svn_stream_set_line_transformer_callback(original_text, - remove_leading_char_transformer); /* Create a stream which returns the modified hunk text. */ SVN_ERR(svn_io_file_open(&f, patch->path, flags, APR_OS_DEFAULT, @@ -525,15 +668,11 @@ parse_next_hunk(svn_hunk_t **hunk, modified_text = svn_stream_from_aprfile_range_readonly(f, FALSE, start, end, result_pool); - svn_stream_set_line_filter_callback(modified_text, modified_line_filter); - svn_stream_set_line_transformer_callback(modified_text, - remove_leading_char_transformer); - /* Set the hunk's texts. */ + (*hunk)->diff_text = diff_text; + (*hunk)->reverse = reverse; if (reverse) { - svn_stream_set_line_transformer_callback(diff_text, - reverse_diff_transformer); (*hunk)->original_text = modified_text; (*hunk)->modified_text = original_text; } Modified: subversion/trunk/subversion/libsvn_subr/stream.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/stream.c?rev=961969&r1=961968&r2=961969&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_subr/stream.c (original) +++ subversion/trunk/subversion/libsvn_subr/stream.c Thu Jul 8 22:42:46 2010 @@ -53,8 +53,6 @@ struct svn_stream_t { svn_io_reset_fn_t reset_fn; svn_io_mark_fn_t mark_fn; svn_io_seek_fn_t seek_fn; - svn_io_line_filter_cb_t line_filter_cb; - svn_io_line_transformer_cb_t line_transformer_cb; }; @@ -73,8 +71,6 @@ svn_stream_create(void *baton, apr_pool_ stream->reset_fn = NULL; stream->mark_fn = NULL; stream->seek_fn = NULL; - stream->line_filter_cb = NULL; - stream->line_transformer_cb = NULL; return stream; } @@ -123,21 +119,6 @@ svn_stream_set_seek(svn_stream_t *stream stream->seek_fn = seek_fn; } -void -svn_stream_set_line_filter_callback(svn_stream_t *stream, - svn_io_line_filter_cb_t line_filter_cb) -{ - stream->line_filter_cb = line_filter_cb; -} - -void -svn_stream_set_line_transformer_callback( - svn_stream_t *stream, - svn_io_line_transformer_cb_t line_transformer_cb) -{ - stream->line_transformer_cb = line_transformer_cb; -} - svn_error_t * svn_stream_read(svn_stream_t *stream, char *buffer, apr_size_t *len) { @@ -233,45 +214,6 @@ svn_stream_printf_from_utf8(svn_stream_t return svn_stream_write(stream, translated, &len); } -/* If a line filter callback was set on STREAM, invoke it on LINE, - * and indicate in *FILTERED whether the line should be filtered. - * If no line filter callback was set on STREAM, just set *FILTERED to FALSE. - */ -static svn_error_t * -line_filter(svn_stream_t *stream, svn_boolean_t *filtered, const char *line, - apr_pool_t *pool) -{ - apr_pool_t *scratch_pool; - - if (! stream->line_filter_cb) - { - *filtered = FALSE; - return SVN_NO_ERROR; - } - - scratch_pool = svn_pool_create(pool); - SVN_ERR(stream->line_filter_cb(filtered, line, stream->baton, scratch_pool)); - svn_pool_destroy(scratch_pool); - return SVN_NO_ERROR; -} - -/* Run the line transformer callback of STREAM with LINE as input, - * and expect the transformation result to be returned in BUF, - * allocated in POOL. */ -static svn_error_t * -line_transformer(svn_stream_t *stream, svn_stringbuf_t **buf, - const char *line, apr_pool_t *pool, apr_pool_t *scratch_pool) -{ - *buf = NULL; /* so we can assert that the callback has set it non-null */ - SVN_ERR(stream->line_transformer_cb(buf, line, stream->baton, - pool, scratch_pool)); - - /* Die if the line transformer didn't provide any output. */ - SVN_ERR_ASSERT(*buf); - - return SVN_NO_ERROR; -} - /* Scan STREAM for an end-of-line indicatior, and return it in *EOL. * Set *EOL to NULL if the stream runs out before an end-of-line indicator * is found. */ @@ -318,84 +260,58 @@ stream_readline(svn_stringbuf_t **string apr_pool_t *pool) { svn_stringbuf_t *str; - apr_pool_t *iterpool; - svn_boolean_t filtered; const char *eol_str; + apr_size_t numbytes; + const char *match; + char c; + + /* Since we're reading one character at a time, let's at least + optimize for the 90% case. 90% of the time, we can avoid the + stringbuf ever having to realloc() itself if we start it out at + 80 chars. */ + str = svn_stringbuf_create_ensure(80, pool); - *eof = FALSE; - - iterpool = svn_pool_create(pool); - do + if (detect_eol) { - apr_size_t numbytes; - const char *match; - char c; - - svn_pool_clear(iterpool); - - /* Since we're reading one character at a time, let's at least - optimize for the 90% case. 90% of the time, we can avoid the - stringbuf ever having to realloc() itself if we start it out at - 80 chars. */ - str = svn_stringbuf_create_ensure(80, iterpool); - - if (detect_eol) + SVN_ERR(scan_eol(&eol_str, stream, pool)); + if (eol) + *eol = eol_str; + if (! eol_str) { - SVN_ERR(scan_eol(&eol_str, stream, iterpool)); - if (eol) - *eol = eol_str; - if (! eol_str) - { - /* No newline until EOF, EOL_STR can be anything. */ - eol_str = APR_EOL_STR; - } + /* No newline until EOF, EOL_STR can be anything. */ + eol_str = APR_EOL_STR; } - else - eol_str = *eol; + } + else + eol_str = *eol; - /* Read into STR up to and including the next EOL sequence. */ - match = eol_str; + /* Read into STR up to and including the next EOL sequence. */ + match = eol_str; + while (*match) + { numbytes = 1; - while (*match) + SVN_ERR(svn_stream_read(stream, &c, &numbytes)); + if (numbytes != 1) { - SVN_ERR(svn_stream_read(stream, &c, &numbytes)); - if (numbytes != 1) - { - /* a 'short' read means the stream has run out. */ - *eof = TRUE; - /* We know we don't have a whole EOL sequence, but ensure we - * don't chop off any partial EOL sequence that we may have. */ - match = eol_str; - /* Process this short (or empty) line just like any other - * except with *EOF set. */ - break; - } - - if (c == *match) - match++; - else - match = eol_str; - - svn_stringbuf_appendbytes(str, &c, 1); + /* a 'short' read means the stream has run out. */ + *eof = TRUE; + if (eol) + *eol = NULL; + *stringbuf = str; + return SVN_NO_ERROR; } - svn_stringbuf_chop(str, match - eol_str); + if (c == *match) + match++; + else + match = eol_str; - SVN_ERR(line_filter(stream, &filtered, str->data, iterpool)); + svn_stringbuf_appendbytes(str, &c, 1); } - while (filtered && ! *eof); - /* Not destroying the iterpool just yet since we still need STR - * which is allocated in it. */ - - if (filtered) - *stringbuf = svn_stringbuf_create_ensure(0, pool); - else if (stream->line_transformer_cb) - SVN_ERR(line_transformer(stream, stringbuf, str->data, pool, iterpool)); - else - *stringbuf = svn_stringbuf_dup(str, pool); - /* Done. RIP iterpool. */ - svn_pool_destroy(iterpool); + *eof = FALSE; + svn_stringbuf_chop(str, match - eol_str); + *stringbuf = str; return SVN_NO_ERROR; } Modified: subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c?rev=961969&r1=961968&r2=961969&view=diff ============================================================================== --- subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c (original) +++ subversion/trunk/subversion/tests/libsvn_diff/parse-diff-test.c Thu Jul 8 22:42:46 2010 @@ -195,15 +195,18 @@ create_patch_file(apr_file_t **patch_fil return SVN_NO_ERROR; } -/* Check that CONTENT equals what's inside EXPECTED. */ +/* Check that reading a line from HUNK equals what's inside EXPECTED. + * If ORIGINAL is TRUE, read the original hunk text; else, read the + * modified hunk text. */ static svn_error_t * -check_content(svn_stream_t *content, const char *expected, apr_pool_t *pool) +check_content(svn_hunk_t *hunk, svn_boolean_t original, + const char *expected, apr_pool_t *pool) { svn_stream_t *exp; svn_stringbuf_t *exp_buf; - svn_stringbuf_t *content_buf; + svn_stringbuf_t *hunk_buf; svn_boolean_t exp_eof; - svn_boolean_t content_eof; + svn_boolean_t hunk_eof; exp = svn_stream_from_string(svn_string_create(expected, pool), pool); @@ -211,18 +214,23 @@ check_content(svn_stream_t *content, con while (TRUE) { SVN_ERR(svn_stream_readline(exp, &exp_buf, NL, &exp_eof, pool)); - SVN_ERR(svn_stream_readline(content, &content_buf, NL, &content_eof, - pool)); - SVN_TEST_ASSERT(exp_eof == content_eof); + if (original) + SVN_ERR(svn_diff_hunk_readline_original_text(hunk, &hunk_buf, NULL, + &hunk_eof, pool, pool)); + else + SVN_ERR(svn_diff_hunk_readline_modified_text(hunk, &hunk_buf, NULL, + &hunk_eof, pool, pool)); + + SVN_TEST_ASSERT(exp_eof == hunk_eof); if (exp_eof) break; - if (strcmp(exp_buf->data, content_buf->data)) + if (strcmp(exp_buf->data, hunk_buf->data)) return svn_error_createf(SVN_ERR_TEST_FAILED, NULL, "Expected '%s' but was '%s'", exp_buf->data, - content_buf->data); + hunk_buf->data); } - SVN_TEST_ASSERT(content_buf->len == 0); + SVN_TEST_ASSERT(hunk_buf->len == 0); return SVN_NO_ERROR; } @@ -247,8 +255,6 @@ test_parse_unidiff(apr_pool_t *pool) svn_patch_t *patch; svn_hunk_t *hunk; apr_off_t pos; - svn_stream_t *original_text; - svn_stream_t *modified_text; svn_pool_clear(iterpool); @@ -267,24 +273,11 @@ test_parse_unidiff(apr_pool_t *pool) SVN_TEST_ASSERT(patch->hunks->nelts == 1); hunk = APR_ARRAY_IDX(patch->hunks, 0, svn_hunk_t *); - if (reverse) - { - /* Hunk texts come out of the parser inverted, - * so this inverts them a second time. */ - original_text = hunk->modified_text; - modified_text = hunk->original_text; - } - else - { - original_text = hunk->original_text; - modified_text = hunk->modified_text; - } - - SVN_ERR(check_content(original_text, + SVN_ERR(check_content(hunk, ! reverse, "This is the file 'gamma'." NL, pool)); - SVN_ERR(check_content(modified_text, + SVN_ERR(check_content(hunk, reverse, "This is the file 'gamma'." NL "some more bytes to 'gamma'" NL, pool)); @@ -306,25 +299,12 @@ test_parse_unidiff(apr_pool_t *pool) SVN_TEST_ASSERT(patch->hunks->nelts == 1); hunk = APR_ARRAY_IDX(patch->hunks, 0, svn_hunk_t *); - if (reverse) - { - /* Hunk texts come out of the parser inverted, - * so this inverts them a second time. */ - original_text = hunk->modified_text; - modified_text = hunk->original_text; - } - else - { - original_text = hunk->original_text; - modified_text = hunk->modified_text; - } - - SVN_ERR(check_content(original_text, + SVN_ERR(check_content(hunk, ! reverse, "This is the file 'gamma'." NL "some less bytes to 'gamma'" NL, pool)); - SVN_ERR(check_content(modified_text, + SVN_ERR(check_content(hunk, reverse, "This is the file 'gamma'." NL, pool)); @@ -370,11 +350,11 @@ test_parse_git_diff(apr_pool_t *pool) hunk = APR_ARRAY_IDX(patch->hunks, 0, svn_hunk_t *); - SVN_ERR(check_content(hunk->original_text, + SVN_ERR(check_content(hunk, TRUE, "This is the file 'gamma'." NL, pool)); - SVN_ERR(check_content(hunk->modified_text, + SVN_ERR(check_content(hunk, FALSE, "This is the file 'gamma'." NL "some more bytes to 'gamma'" NL, pool)); @@ -432,11 +412,11 @@ test_parse_git_tree_and_text_diff(apr_po hunk = APR_ARRAY_IDX(patch->hunks, 0, svn_hunk_t *); - SVN_ERR(check_content(hunk->original_text, + SVN_ERR(check_content(hunk, TRUE, "This is the file 'iota'." NL, pool)); - SVN_ERR(check_content(hunk->modified_text, + SVN_ERR(check_content(hunk, FALSE, "This is the file 'iota'." NL "some more bytes to 'iota'" NL, pool)); @@ -454,11 +434,11 @@ test_parse_git_tree_and_text_diff(apr_po hunk = APR_ARRAY_IDX(patch->hunks, 0, svn_hunk_t *); - SVN_ERR(check_content(hunk->original_text, + SVN_ERR(check_content(hunk, TRUE, "This is the file 'mu'." NL, pool)); - SVN_ERR(check_content(hunk->modified_text, + SVN_ERR(check_content(hunk, FALSE, "This is the file 'mu'." NL "some more bytes to 'mu'" NL, pool)); @@ -490,11 +470,11 @@ test_bad_git_diff_headers(apr_pool_t *po hunk = APR_ARRAY_IDX(patch->hunks, 0, svn_hunk_t *); - SVN_ERR(check_content(hunk->original_text, + SVN_ERR(check_content(hunk, TRUE, "This is the file 'iota'." NL, pool)); - SVN_ERR(check_content(hunk->modified_text, + SVN_ERR(check_content(hunk, FALSE, "This is the file 'iota'." NL "some more bytes to 'iota'" NL, pool)); @@ -530,11 +510,11 @@ test_parse_property_diff(apr_pool_t *poo SVN_TEST_ASSERT(hunks->nelts == 1); hunk = APR_ARRAY_IDX(hunks, 0 , svn_hunk_t *); - SVN_ERR(check_content(hunk->original_text, + SVN_ERR(check_content(hunk, TRUE, "", pool)); - SVN_ERR(check_content(hunk->modified_text, + SVN_ERR(check_content(hunk, FALSE, "value" NL, pool)); @@ -543,11 +523,11 @@ test_parse_property_diff(apr_pool_t *poo SVN_TEST_ASSERT(hunks->nelts == 1); hunk = APR_ARRAY_IDX(hunks, 0 , svn_hunk_t *); - SVN_ERR(check_content(hunk->original_text, + SVN_ERR(check_content(hunk, TRUE, "value" NL, pool)); - SVN_ERR(check_content(hunk->modified_text, + SVN_ERR(check_content(hunk, FALSE, "", pool)); @@ -556,11 +536,11 @@ test_parse_property_diff(apr_pool_t *poo SVN_TEST_ASSERT(hunks->nelts == 1); hunk = APR_ARRAY_IDX(hunks, 0 , svn_hunk_t *); - SVN_ERR(check_content(hunk->original_text, + SVN_ERR(check_content(hunk, TRUE, "value" NL, pool)); - SVN_ERR(check_content(hunk->modified_text, + SVN_ERR(check_content(hunk, FALSE, "new value" NL, pool)); @@ -592,11 +572,11 @@ test_parse_property_and_text_diff(apr_po /* Check contents of text hunk */ hunk = APR_ARRAY_IDX(patch->hunks, 0, svn_hunk_t *); - SVN_ERR(check_content(hunk->original_text, + SVN_ERR(check_content(hunk, TRUE, "This is the file 'iota'." NL, pool)); - SVN_ERR(check_content(hunk->modified_text, + SVN_ERR(check_content(hunk, FALSE, "This is the file 'iota'." NL "some more bytes to 'iota'" NL, pool)); @@ -606,11 +586,11 @@ test_parse_property_and_text_diff(apr_po SVN_TEST_ASSERT(hunks->nelts == 1); hunk = APR_ARRAY_IDX(hunks, 0 , svn_hunk_t *); - SVN_ERR(check_content(hunk->original_text, + SVN_ERR(check_content(hunk, TRUE, "", pool)); - SVN_ERR(check_content(hunk->modified_text, + SVN_ERR(check_content(hunk, FALSE, "value" NL, pool)); 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=961969&r1=961968&r2=961969&view=diff ============================================================================== --- subversion/trunk/subversion/tests/libsvn_subr/stream-test.c (original) +++ subversion/trunk/subversion/tests/libsvn_subr/stream-test.c Thu Jul 8 22:42:46 2010 @@ -307,142 +307,6 @@ test_stream_range(apr_pool_t *pool) return SVN_NO_ERROR; } -/* An implementation of svn_io_line_filter_cb_t */ -static svn_error_t * -line_filter(svn_boolean_t *filtered, const char *line, void *baton, - apr_pool_t *scratch_pool) -{ - *filtered = strchr(line, '!') != NULL; - return SVN_NO_ERROR; -} - -static svn_error_t * -test_stream_line_filter(apr_pool_t *pool) -{ - static const char *lines[4] = {"Not filtered.", "Filtered!", - "Not filtered either.", "End of the lines!"}; - svn_string_t *string; - svn_stream_t *stream; - svn_stringbuf_t *line; - svn_boolean_t eof; - - string = svn_string_createf(pool, "%s\n%s\n%s\n%s", lines[0], lines[1], - lines[2], lines[3]); - stream = svn_stream_from_string(string, pool); - - svn_stream_set_line_filter_callback(stream, line_filter); - - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_STRING_ASSERT(line->data, lines[0]); - /* line[1] should be filtered */ - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_STRING_ASSERT(line->data, lines[2]); - - /* The last line should also be filtered, and the resulting - * stringbuf should be empty. */ - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_ASSERT(eof && svn_stringbuf_isempty(line)); - - return SVN_NO_ERROR; -} - -/* An implementation of svn_io_line_transformer_cb_t */ -static svn_error_t * -line_transformer(svn_stringbuf_t **buf, const char *line, void *baton, - apr_pool_t *result_pool, apr_pool_t *scratch_pool) -{ - int i, len = strlen(line); - char *temp = apr_palloc(scratch_pool, len + 1 ); - - for (i = 0; i < len; i++) - { - temp[i] = line[len - 1 - i]; - } - - temp[len] = '\0'; - - *buf = svn_stringbuf_create(temp, result_pool); - - return SVN_NO_ERROR; -} - -static svn_error_t * -test_stream_line_transformer(apr_pool_t *pool) -{ - static const char *lines[4] = {"gamma", "", - "iota", "!"}; - - static const char *inv_lines[4] = {"ammag", "", - "atoi", "!"}; - svn_string_t *string; - svn_stream_t *stream; - svn_stringbuf_t *line; - svn_boolean_t eof; - - string = svn_string_createf(pool, "%s\n%s\n%s\n%s", lines[0], lines[1], - lines[2], lines[3]); - - stream = svn_stream_from_string(string, pool); - - svn_stream_set_line_transformer_callback(stream, line_transformer); - - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_STRING_ASSERT(line->data, inv_lines[0]); - - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_STRING_ASSERT(line->data, inv_lines[1]); - - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_STRING_ASSERT(line->data, inv_lines[2]); - - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_STRING_ASSERT(line->data, inv_lines[3]); - - /* We should have reached eof and the stringbuf should be emtpy. */ - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_ASSERT(eof && svn_stringbuf_isempty(line)); - - return SVN_NO_ERROR; -} - -static svn_error_t * -test_stream_line_filter_and_transformer(apr_pool_t *pool) -{ - static const char *lines[4] = {"!gamma", "", - "iota", "!"}; - - static const char *inv_lines[4] = {"ammag", "", - "atoi", "!"}; - svn_string_t *string; - svn_stream_t *stream; - svn_stringbuf_t *line; - svn_boolean_t eof; - - string = svn_string_createf(pool, "%s\n%s\n%s\n%s", lines[0], lines[1], - lines[2], lines[3]); - - stream = svn_stream_from_string(string, pool); - - svn_stream_set_line_filter_callback(stream, line_filter); - - svn_stream_set_line_transformer_callback(stream, line_transformer); - - /* Line one should be filtered. */ - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_STRING_ASSERT(line->data, inv_lines[1]); - - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_STRING_ASSERT(line->data, inv_lines[2]); - - /* The last line should also be filtered, and the resulting - * stringbuf should be empty. */ - svn_stream_readline(stream, &line, "\n", &eof, pool); - SVN_TEST_ASSERT(eof && svn_stringbuf_isempty(line)); - - return SVN_NO_ERROR; - -} - static svn_error_t * test_stream_tee(apr_pool_t *pool) { @@ -647,12 +511,6 @@ struct svn_test_descriptor_t test_funcs[ "test compressed streams"), SVN_TEST_PASS2(test_stream_range, "test streams reading from range of file"), - SVN_TEST_PASS2(test_stream_line_filter, - "test stream line filtering"), - SVN_TEST_PASS2(test_stream_line_transformer, - "test stream line transforming"), - SVN_TEST_PASS2(test_stream_line_filter_and_transformer, - "test stream line filtering and transforming"), SVN_TEST_PASS2(test_stream_tee, "test 'tee' streams"), SVN_TEST_PASS2(test_stream_seek_file,
