When I wrote svn_txdelta_run() a while back, I seem to recall that it
could be used in the FS library, if a couple function calls get
collapsed. i.e part of the process is started in one function, then
completed in another function. But overall, those two pieces do what
svn_txdelta_run() does. And... txdelta_run allows for skipping the
checksum thing.
Is this code amenable to using svn_txdelta_run? That would simplify
things, and we wouldn't need another entry point.
Cheers,
-g
On Tue, Apr 6, 2010 at 14:24, Stefan Fuhrman
<stefanfuhrmann_at_alice-dsl.de> wrote:
> Hi devs,
>
> FSFS calculates and checks MD5 in rep_read_contents.
> So, there is no reason so calculate it a second time in the
> delta stream svn_fs_fs__get_file_delta_stream returns.
> However, it gets calculated there "accidentally" and not
> used later.
>
> This patch adds a variant of svn_txdelta() that will not
> calculate the checksum. It uses that new function in FSFS.
>
> -- Stefan^2.
>
> [[[
> Eliminate superfluous checksum calculation when
> reading deltas from FSFS.
>
> * subversion/include/svn_delta.h
> (svn_txdelta_unchecked): declare new function
>
> * subversion/libsvn_delta/text_delta.c
> add include for the _() nls macro
> (txdelta_baton): extend commentary
> (txdelta_no_digest, svn_txdelta_unchecked):
> new functions
> (svn_txdelta_send_stream): make sure to detect
> cases where svn_txdelta_unchecked was used
> but not appropriate
>
> * subversion/libsvn_fs_fs/fs_fs.c
> (svn_fs_fs__get_file_delta_stream): return
> a non-checksumming stream.
>
> patch by stefanfuhrmann < at > alice-dsl.de
> ]]]
>
> Index: subversion/include/svn_delta.h
> ===================================================================
> --- subversion/include/svn_delta.h (revision 930220)
> +++ subversion/include/svn_delta.h (working copy)
> @@ -353,7 +353,22 @@
> svn_stream_t *target,
> apr_pool_t *pool);
>
> +/** Just like @ref svn_txdelta except that no checksum is being created.
> + * When calling @ref svn_txdelta_md5_digest with @a *stream, it will
> + * return @c NULL.
> + *
> + * Use this function only if the @a source stream comes from a trustworthy
> + * source as e.g. an in-process FS driver.
> + *
> + * @since New in 1.7.
> + */
> +void
> +svn_txdelta_unchecked(svn_txdelta_stream_t **stream,
> + svn_stream_t *source,
> + svn_stream_t *target,
> + apr_pool_t *pool);
>
> +
> /**
> * Return a writable stream which, when fed target data, will send
> * delta windows to @a handler/@a handler_baton which transform the
> Index: subversion/libsvn_delta/text_delta.c
> ===================================================================
> --- subversion/libsvn_delta/text_delta.c (revision 930220)
> +++ subversion/libsvn_delta/text_delta.c (working copy)
> @@ -32,6 +32,7 @@
> #include "svn_io.h"
> #include "svn_pools.h"
> #include "svn_checksum.h"
> +#include "svn_private_config.h"
>
> #include "delta.h"
>
> @@ -57,7 +58,7 @@
> svn_filesize_t pos; /* Offset of next read in source file. */
> char *buf; /* Buffer for input data. */
>
> - svn_checksum_ctx_t *context; /* Context for computing the checksum. */
> + svn_checksum_ctx_t *context; /* Context for computing the checksum. May
> be NULL. */
> svn_checksum_t *checksum; /* If non-NULL, the checksum of TARGET. */
>
> apr_pool_t *result_pool; /* For results (e.g. checksum) */
> @@ -368,6 +369,13 @@
> }
>
>
> +static const unsigned char *
> +txdelta_no_digest(void *baton)
> +{
> + return NULL;
> +}
> +
> +
> svn_error_t *
> svn_txdelta_run(svn_stream_t *source,
> svn_stream_t *target,
> @@ -440,7 +448,26 @@
> txdelta_md5_digest, pool);
> }
>
> +void
> +svn_txdelta_unchecked(svn_txdelta_stream_t **stream,
> + svn_stream_t *source,
> + svn_stream_t *target,
> + apr_pool_t *pool)
> +{
> + struct txdelta_baton *b = apr_pcalloc(pool, sizeof(*b));
>
> + b->source = source;
> + b->target = target;
> + b->more_source = TRUE;
> + b->more = TRUE;
> + b->buf = apr_palloc(pool, 2 * SVN_DELTA_WINDOW_SIZE);
> + b->result_pool = pool;
> +
> + *stream = svn_txdelta_stream_create(b, txdelta_next_window,
> + txdelta_no_digest, pool);
> +}
> +
> +
>
> /* Functions for implementing a "target push" delta. */
>
>
> @@ -814,7 +859,14 @@
> {
> const unsigned char *result_md5;
> result_md5 = svn_txdelta_md5_digest(txstream);
> - /* Since err is null, result_md5 "cannot" be null. */
> +
> + /* Don't call this function for streams created with
> + svn_txdelta_unchecked. Or txdelta_md5_digest was
> + called prematurely. */
> + if (result_md5 == NULL)
> + return svn_error_create(SVN_ERR_INCORRECT_PARAMS, NULL,
> + _("Unchecked or incomplete stream"));
> +
> memcpy(digest, result_md5, APR_MD5_DIGESTSIZE);
> }
>
> return err;
> Index: subversion/libsvn_fs_fs/fs_fs.c
> ===================================================================
> --- subversion/libsvn_fs_fs/fs_fs.c (revision 930220)
> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> @@ -3481,13 +3481,14 @@
> SVN_ERR(svn_io_file_close(rep_state->file, pool));
> }
>
> - /* Read both fulltexts and construct a delta. */
> + /* Read both fulltexts and construct a delta. The checksum for stream_p
> + will not be used by the callers. Thus, don't calculate it. */
> if (source)
> SVN_ERR(read_representation(&source_stream, fs, source->data_rep,
> pool));
> else
> source_stream = svn_stream_empty(pool);
> SVN_ERR(read_representation(&target_stream, fs, target->data_rep, pool));
> - svn_txdelta(stream_p, source_stream, target_stream, pool);
> + svn_txdelta_unchecked(stream_p, source_stream, target_stream, pool);
>
> return SVN_NO_ERROR;
> }
>
>
Received on 2010-04-06 20:52:27 CEST