[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r38973 - in trunk/subversion: include libsvn_subr tests/libsvn_subr

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Fri, 28 Aug 2009 03:57:32 +0300 (Jerusalem Daylight Time)

Stefan Sperling wrote on Thu, 27 Aug 2009 at 13:35 -0700:
> Log:
> Create a new callback for svn_stream_readline() that can perform
> arbitrary transformations on a line before it is returned to the
> caller of svn_stream_readline().
>
...
> +++ trunk/subversion/libsvn_subr/stream.c Thu Aug 27 13:35:11 2009 (r38973)
> @@ -207,6 +217,25 @@ line_filter(svn_stream_t *stream, svn_bo
> return SVN_NO_ERROR;
> }
>
> +/* Run the line transforter 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;
> +
> + scratch_pool = svn_pool_create(pool);
> + SVN_ERR(stream->line_transformer_cb(buf, line, pool, scratch_pool));
> + svn_pool_destroy(scratch_pool);
> +

So, we create/destroy a pool for *every single line* in the stream?
(when a transformation callback is set)

Isn't that too expensive? Perhaps this function (line_transformer)
should simply take two pool arguments like everyone else?

> Modified: trunk/subversion/tests/libsvn_subr/stream-test.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/libsvn_subr/stream-test.c?pathrev=38973&r1=38972&r2=38973
> ==============================================================================
> --- trunk/subversion/tests/libsvn_subr/stream-test.c Thu Aug 27 13:29:01 2009 (r38972)
> +++ trunk/subversion/tests/libsvn_subr/stream-test.c Thu Aug 27 13:35:11 2009 (r38973)
> @@ -345,6 +345,105 @@ test_stream_line_filter(apr_pool_t *pool
> 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,
> + apr_pool_t *result_pool, apr_pool_t *scratch_pool)

Reverses a string. The comment could say that?

> +{
> + 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;
> +}

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2388082
Received on 2009-08-28 07:39:16 CEST

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