On Mon, Aug 18, 2008 at 1:05 PM, <hwright_at_tigris.org> wrote:
>...
> * If the filesystem does not have a prerecorded checksum for @a path,
> - * do not calculate a checksum dynamically, just put all 0's into @a
> - * digest. (By convention, the all-zero checksum is considered to
> - * match any checksum.)
> + * and @a force is not TRUE, do not calculate a checksum dynamically, just
> + * put NULL into @a checksum. (By convention, the NULL checksum is
> + * considered to match any checksum.)
Nit: "does not have a prerecorded checksum [of the right kind]"
The function will recompute a checksum if it is the wrong kind, and
force is set.
>...
> +++ trunk/subversion/libsvn_fs/fs-loader.h Mon Aug 18 13:05:09 2008 (r32527)
> @@ -273,9 +273,8 @@ typedef struct root_vtable_t
> /* Files */
> svn_error_t *(*file_length)(svn_filesize_t *length_p, svn_fs_root_t *root,
> const char *path, apr_pool_t *pool);
> - svn_error_t *(*file_md5_checksum)(unsigned char digest[],
> - svn_fs_root_t *root,
> - const char *path, apr_pool_t *pool);
> + svn_error_t *(*file_checksum)(svn_checksum_t **checksum, svn_fs_root_t *root,
> + const char *path, apr_pool_t *pool);
Should there be a "kind" in this API?
If not, then how is the API defined? "Whatever checksum is available,
otherwise a SHA1 checksum." ?
>...
> +++ trunk/subversion/libsvn_fs_base/dag.c Mon Aug 18 13:05:09 2008 (r32527)
>...
> - if (checksum)
> - {
> - unsigned char digest[APR_MD5_DIGESTSIZE];
> - const char *hex;
> -
> - SVN_ERR(svn_fs_base__rep_contents_checksum
> - (digest, fs, noderev->edit_key, trail, pool));
> -
> - hex = svn_md5_digest_to_cstring_display(digest, pool);
> - if (strcmp(checksum, hex) != 0)
> - return svn_error_createf
> - (SVN_ERR_CHECKSUM_MISMATCH,
> - NULL,
> - _("Checksum mismatch, rep '%s':\n"
> - " expected: %s\n"
> - " actual: %s\n"),
> - noderev->edit_key, checksum, hex);
> - }
> + /* Get our representation's checksum. */
> + SVN_ERR(svn_fs_base__rep_contents_checksum(&rep_checksum, fs,
> + noderev->edit_key, trail, pool));
Woah. This will always compute it now, which is quite different from
before. I think you should retain the optional behavior of "if they
provided a checksum, *then* compute it and compare it."
>...
> @@ -1416,6 +1412,18 @@ svn_fs_base__dag_deltify(dag_node_t *tar
> }
>
>
> +svn_error_t *
> +svn_fs_base__dag_index_checksums(dag_node_t *node,
> + trail_t *trail,
> + apr_pool_t *pool)
> +{
> + node_revision_t *node_rev;
> +
> + SVN_ERR(svn_fs_bdb__get_node_revision(&node_rev, trail->fs, node->id,
> + trail, pool));
> + return SVN_NO_ERROR;
> +}
>...
> +++ trunk/subversion/libsvn_fs_base/dag.h Mon Aug 18 13:05:09 2008 (r32527)
>...
> -/* Put the recorded MD5 checksum of FILE into DIGEST, as part of
> - * TRAIL. DIGEST must point to APR_MD5_DIGESTSIZE bytes of storage.
> +/* Put the recorded checksum of FILE into CHECKSUM, as part of
> + * TRAIL.
> *
> * If no stored checksum is available, do not calculate the checksum,
> * just put all 0's into DIGEST.
Doesn't it set *checksum to NULL?
>...
> +++ trunk/subversion/libsvn_fs_base/reps-strings.c Mon Aug 18 13:05:09 2008 (r32527)
>...
> @@ -797,22 +799,20 @@ svn_fs_base__rep_contents(svn_string_t *
> /* Just the standard paranoia. */
> {
> representation_t *rep;
> - apr_md5_ctx_t md5_context;
> - unsigned char checksum[APR_MD5_DIGESTSIZE];
> + svn_checksum_t *checksum;
>
> - apr_md5_init(&md5_context);
> - apr_md5_update(&md5_context, str->data, str->len);
> - apr_md5_final(checksum, &md5_context);
> + SVN_ERR(svn_checksum(&checksum, svn_checksum_md5, str->data, str->len,
Leave a ### comment that this should switch to svn_checksum_sha1?
>...
> +++ trunk/subversion/libsvn_fs_base/reps-strings.h Mon Aug 18 13:05:09 2008 (r32527)
> @@ -76,12 +76,12 @@ svn_error_t *svn_fs_base__rep_contents_s
> apr_pool_t *pool);
>
>
> -/* Put into DIGEST the MD5 checksum for REP_KEY in FS, as part of TRAIL.
> - This is the prerecorded checksum for the rep's contents' fulltext.
> - If no checksum is available, do not calculate one dynamically, just
> - put all 0's into DIGEST. (By convention, the all-zero checksum is
> - considered to match any checksum.) */
> -svn_error_t *svn_fs_base__rep_contents_checksum(unsigned char digest[],
> +/* Put into CHECKSUM the checksum for REP_KEY in FS, as part of TRAIL. This
> + is the prerecorded checksum for the rep's contents' fulltext. If no checksum
> + of this type is available, do not calculate one dynamically, just put NULL
"of this type" ?? There is no requested type. Should the kind be an
argument? Or will this function always produce a checksum of kind X?
>...
> +++ trunk/subversion/libsvn_fs_base/tree.c Mon Aug 18 13:05:09 2008 (r32527)
>...
> + {
> + tb->base_checksum = svn_checksum_create(svn_checksum_md5, pool);
> + SVN_ERR(svn_checksum_parse_hex(tb->base_checksum, base_checksum));
> + }
> else
> tb->base_checksum = NULL;
>
> if (result_checksum)
> - tb->result_checksum = apr_pstrdup(pool, result_checksum);
> + {
> + tb->result_checksum = svn_checksum_create(svn_checksum_md5, pool);
> + SVN_ERR(svn_checksum_parse_hex(tb->result_checksum, result_checksum));
Won't it always be a pair of calls like this? Shouldn't parse_hex()
just be a constructor, rather than a modifier?
>...
> +++ trunk/subversion/libsvn_fs_base/util/fs_skels.c Mon Aug 18 13:05:09 2008 (r32527)
>...
> +++ trunk/subversion/libsvn_fs_fs/tree.c Mon Aug 18 13:05:09 2008 (r32527)
>...
> +fs_file_checksum(svn_checksum_t **checksum,
> + svn_fs_root_t *root,
> + const char *path,
> + apr_pool_t *pool)
> {
> dag_node_t *file;
>
> SVN_ERR(get_dag(&file, root, path, pool));
> - return svn_fs_fs__dag_file_checksum(digest, file, pool);
> +
> + *checksum = svn_checksum_create(svn_checksum_md5, pool);
Leave a ### marker to change this, or take the kind as an argument.
>...
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-08-18 23:10:46 CEST