On Thu, Sep 25, 2008 at 9:35 PM, <hwright_at_tigris.org> wrote:
>...
> +++ branches/fs-rep-sharing/subversion/libsvn_fs_base/dag.c Thu Sep 25 21:35:28 2008 (r33306)
>...
> @@ -1245,8 +1245,10 @@ svn_fs_base__dag_finalize_edits(dag_node
> SVN_ERR(svn_fs_base__rep_contents_checksum(&rep_checksum, fs,
> noderev->edit_key, trail, pool));
>
> - /* If our caller provided a checksum to compare, do so. */
> - if (checksum && !svn_checksum_match(checksum, rep_checksum))
> + /* If our caller provided a checksum of the right kind to compare, do so. */
> + if (checksum
> + && checksum->kind == rep_checksum->kind
> + && !svn_checksum_match(checksum, rep_checksum))
Why check the kind? Doesn't svn_checksum_match() do that? And if not,
then it *should*.
>...
> +++ branches/fs-rep-sharing/subversion/libsvn_fs_base/reps-strings.c Thu Sep 25 21:35:28 2008 (r33306)
>...
> @@ -638,12 +638,19 @@ struct rep_read_baton
>...
> + /* MD5 checksum context. Initialized when the baton is created, updated as
> + we read data, and finalized when the stream is closed. */
> + svn_checksum_ctx_t *sha1_checksum_ctx;
Bad comment. That's the SHA1 checksum context.
And do we really need *both* forms? Why?
>...
> +++ branches/fs-rep-sharing/subversion/libsvn_fs_base/tree.c Thu Sep 25 21:35:28 2008 (r33306)
> @@ -3672,7 +3672,12 @@ txn_body_apply_textdelta(void *baton, tr
> contents, in other words, the base text. */
> SVN_ERR(svn_fs_base__dag_file_checksum(&checksum, tb->node,
> trail, trail->pool));
> - if (!svn_checksum_match(tb->base_checksum, checksum))
> + /* TODO: This only compares checksums if they are the same kind, but
> + we're calculating both SHA1 and MD5 checksums somewhere in
> + reps-strings.c. Could we keep them both around somehow so this
> + check could be more comprehensive? */
> + if (tb->base_checksum->kind == checksum->kind
> + && !svn_checksum_match(tb->base_checksum, checksum))
> return svn_error_createf
> (SVN_ERR_CHECKSUM_MISMATCH,
Again, why check the kind? Should be able to just call svn_checksum_match().
Note that you may want to consider something like:
svn_checksum_match_either(checksum, md5_desired, sha1_desired)
Then the checksum can be checked against the appropriate kind.
Cheers,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-26 20:53:34 CEST