On Fri, 2009-01-09 at 02:44 -0800, Greg Stein wrote:
> Oh, and __from_digest was made private because we didn't *want* to
> make it public. That function is an interim until other APIs move from
> the old-style digest over to new checksums. (e.g. the editor API)
> Once those APIs are shifted, then we won't need __from_digest except
> to deal with backwards compat issues.
>
> On Fri, Jan 9, 2009 at 02:42, Greg Stein <gstein_at_gmail.com> wrote:
> > Why this extra complexity? This is a simple type with simple
> > semantics. I don't see the need for all of this, in order to fend off
> > some *potential* future extension that is incompatible. We have lots
> > of public structures with constructor functions, just like
> > svn_checksum_t, so we can easily extend the structure if we choose.
> >
> > What is the rationale for this change?
Ah... thank you for reminding me. This all started with the public
"constructor" function:
[[[
** Allocate, initialize and return a @c svn_checksum_t structure of type
* @a kind. The checksum is allocated in @a pool.
*
* @since New in 1.6.
*/
svn_checksum_t *
svn_checksum_create(svn_checksum_kind_t kind,
apr_pool_t *pool);
]]]
That function does not actually construct a complete object, it just
allocates space and fills in one of the fields. The data space in it is
marked read-only ("const"), so the user has to either cast, or allocate
more space and change the pointer.
First thought: just lose the "const". (Referring to:
[[[
typedef struct svn_checksum_t
{
/** The bytes of the checksum digest. */
const unsigned char *digest;
/** The type of the checksum. This should never be changed by consumers
of the APIs. */
svn_checksum_kind_t kind;
} svn_checksum_t;
]]]
)
Losing the "const" would allow all users to fill in the digest data if
they want to, and this might be a good solution (if we want it to be
transparent).
Next I looked at all the uses and saw that in most cases they actually
wanted to construct an initialized checksum - with digest - and the
"svn_checksum__from_digest()" function was the constructor they really
wanted to use. It's easier for the caller to make a single construction
call than to construct an empty object and then fill in its field(s). So
it makes sense for that to be the "normal" constructor.
Then I looked at the set of other checksum API functions, and it looked
like an intention to hide the internals in most cases (e.g. asking for a
hex representation of a checksum object, rather than providing a
separate "convert-a-digest-to-hex" function).
Do these public constructor and accessor functions really constitute
"complexity"? It seems to me that by hiding internal details and making
the user's life easier, they contribute a little simplicity. Not enough,
maybe, for me to object if others want them reverted, though.
- Julian
> > -g
> >
> > On Fri, Jan 9, 2009 at 02:04, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> >> Author: julianfoad
> >> Date: Fri Jan 9 02:04:35 2009
> >> New Revision: 35103
> >>
> >> Log:
> >> In the "checksums" API, make the svn_checksum_t type opaque to help make
> >> future extensions easier, and introduce accessor functions:
> >> svn_checksum_get_kind()
> >> svn_checksum_get_size() (replacing svn_checksum_size())
> >> svn_checksum_get_digest()
> >> to replace direct access.
> >>
> >> Make the constructor "svn_checksum__from_digest()" public as well by
> >> renaming it to "svn_checksum_from_digest()", because it is the logical
> >> complement of the "get" accessors.
> >>
> >> * subversion/include/svn_checksum.h,
> >> subversion/libsvn_subr/checksum.c
> >> (svn_checksum_t): Move the definition from the header to the source file,
> >> leaving just a declaration in the header. Make the "digest" field
> >> non-const and remove all the type casts that were used to write to it
> >> throughout the rest of this source file.
> >> (svn_checksum_size): Rename to ...
> >> (svn_checksum_get_size): ... this and add "const" on the input parameter.
> >> (svn _checksum_get_kind, svn_checksum_get_digest): New functions.
> >> (svn_checksum__from_digest): Rename to ...
> >> (svn_checksum_from_digest): ... this.
> >>
> >> * subversion/libsvn_client/commit.c,
> >> subversion/libsvn_client/commit_util.c,
> >> subversion/libsvn_client/export.c,
> >> subversion/libsvn_delta/text_delta.c,
> >> subversion/libsvn_fs_base/bdb/checksum-reps-table.c,
> >> subversion/libsvn_fs_base/bdb/dbt.c,
> >> subversion/libsvn_fs_base/dag.c,
> >> subversion/libsvn_fs_base/reps-strings.c,
> >> subversion/libsvn_fs_base/tree.c,
> >> subversion/libsvn_fs_base/util/fs_skels.c,
> >> subversion/libsvn_fs_fs/dag.c,
> >> subversion/libsvn_fs_fs/fs_fs.c,
> >> subversion/libsvn_fs/fs-loader.c,
> >> subversion/libsvn_fs_fs/rep-cache.c,
> >> subversion/libsvn_fs_fs/tree.c,
> >> subversion/libsvn_subr/io.c,
> >> subversion/libsvn_subr/stream.c,
> >> subversion/libsvn_wc/adm_crawler.c,
> >> subversion/libsvn_wc/adm_ops.c,
> >> subversion/libsvn_wc/update_editor.c
> >> Adjust for the renaming of svn_checksum__from_digest().
> >> Use the accessor functions instead of direct access.
> >>
> >> * subversion/libsvn_subr/svn_base64.c
> >> (svn_base64_from_checksum):
> >> Use the accessor functions instead of direct access.
> >> (svn_base64_from_md5): Use svn_checksum__from_digest to simplify and avoid
> >> direct write access to a checksum.
> >>
> >> * subversion/tests/libsvn_delta/window-test.c
> >> (stream_window_test): Use svn_checksum_from_digest() to simplify and avoid
> >> direct write access to a checksum.
> >>
> >> Modified:
> >> trunk/subversion/include/svn_checksum.h
> >> trunk/subversion/libsvn_client/commit.c
> >> trunk/subversion/libsvn_client/commit_util.c
> >> trunk/subversion/libsvn_client/export.c
> >> trunk/subversion/libsvn_delta/text_delta.c
> >> trunk/subversion/libsvn_fs/fs-loader.c
> >> trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c
> >> trunk/subversion/libsvn_fs_base/bdb/dbt.c
> >> trunk/subversion/libsvn_fs_base/dag.c
> >> trunk/subversion/libsvn_fs_base/reps-strings.c
> >> trunk/subversion/libsvn_fs_base/tree.c
> >> trunk/subversion/libsvn_fs_base/util/fs_skels.c
> >> trunk/subversion/libsvn_fs_fs/dag.c
> >> trunk/subversion/libsvn_fs_fs/fs_fs.c
> >> trunk/subversion/libsvn_fs_fs/rep-cache.c
> >> trunk/subversion/libsvn_fs_fs/tree.c
> >> trunk/subversion/libsvn_subr/checksum.c
> >> trunk/subversion/libsvn_subr/io.c
> >> trunk/subversion/libsvn_subr/stream.c
> >> trunk/subversion/libsvn_subr/svn_base64.c
> >> trunk/subversion/libsvn_wc/adm_crawler.c
> >> trunk/subversion/libsvn_wc/adm_ops.c
> >> trunk/subversion/libsvn_wc/update_editor.c
> >> trunk/subversion/tests/libsvn_delta/window-test.c
> >>
> >> Modified: trunk/subversion/include/svn_checksum.h
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_checksum.h?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/include/svn_checksum.h Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/include/svn_checksum.h Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -48,15 +48,7 @@ typedef enum
> >> *
> >> * @since New in 1.6.
> >> */
> >> -typedef struct svn_checksum_t
> >> -{
> >> - /** The bytes of the checksum. */
> >> - const unsigned char *digest;
> >> -
> >> - /** The type of the checksum. This should never be changed by consumers
> >> - of the APIs. */
> >> - svn_checksum_kind_t kind;
> >> -} svn_checksum_t;
> >> +typedef struct svn_checksum_t svn_checksum_t;
> >>
> >> /**
> >> * Opaque type for creating checksums of data.
> >> @@ -200,18 +192,34 @@ svn_checksum_final(svn_checksum_t **chec
> >> * @since New in 1.6.
> >> */
> >> apr_size_t
> >> -svn_checksum_size(svn_checksum_t *checksum);
> >> +svn_checksum_get_size(const svn_checksum_t *checksum);
> >>
> >> +/**
> >> + * Return a (read-only) pointer to the binary digest of @a checksum.
> >> + *
> >> + * @since New in 1.6.
> >> + */
> >> +const unsigned char *
> >> +svn_checksum_get_digest(const svn_checksum_t *checksum);
> >> +
> >> +/**
> >> + * Return the checksum kind of @a checksum.
> >> + *
> >> + * @since New in 1.6.
> >> + */
> >> +svn_checksum_kind_t
> >> +svn_checksum_get_kind(const svn_checksum_t *checksum);
> >>
> >> /**
> >> - * Internal function for creating a checksum from a binary digest.
> >> + * Create a checksum from a binary digest @a digest of kind @a kind.
> >> + * Allocate the result in @a pool, making a deep copy of @a digest.
> >> *
> >> * @since New in 1.6
> >> */
> >> svn_checksum_t *
> >> -svn_checksum__from_digest(const unsigned char *digest,
> >> - svn_checksum_kind_t kind,
> >> - apr_pool_t *result_pool);
> >> +svn_checksum_from_digest(const unsigned char *digest,
> >> + svn_checksum_kind_t kind,
> >> + apr_pool_t *result_pool);
> >>
> >>
> >> #ifdef __cplusplus
> >>
> >> Modified: trunk/subversion/libsvn_client/commit.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/commit.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_client/commit.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_client/commit.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -247,8 +247,8 @@ import_file(const svn_delta_editor_t *ed
> >>
> >> /* Finally, close the file. */
> >> text_checksum =
> >> - svn_checksum_to_cstring(svn_checksum__from_digest(digest, svn_checksum_md5,
> >> - pool), pool);
> >> + svn_checksum_to_cstring(svn_checksum_from_digest(digest, svn_checksum_md5,
> >> + pool), pool);
> >>
> >> return editor->close_file(file_baton, text_checksum, pool);
> >> }
> >>
> >> Modified: trunk/subversion/libsvn_client/commit_util.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/commit_util.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_client/commit_util.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_client/commit_util.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -1661,8 +1661,8 @@ svn_client__do_commit(const char *base_u
> >> }
> >> if (checksums)
> >> apr_hash_set(*checksums, item->path, APR_HASH_KEY_STRING,
> >> - svn_checksum__from_digest(digest, svn_checksum_md5,
> >> - apr_hash_pool_get(*checksums)));
> >> + svn_checksum_from_digest(digest, svn_checksum_md5,
> >> + apr_hash_pool_get(*checksums)));
> >> }
> >>
> >> svn_pool_destroy(iterpool);
> >>
> >> Modified: trunk/subversion/libsvn_client/export.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/export.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_client/export.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_client/export.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -723,9 +723,9 @@ close_file(void *file_baton,
> >> if (text_checksum)
> >> {
> >> const char *actual_checksum =
> >> - svn_checksum_to_cstring(svn_checksum__from_digest(fb->text_digest,
> >> - svn_checksum_md5,
> >> - pool), pool);
> >> + svn_checksum_to_cstring(svn_checksum_from_digest(fb->text_digest,
> >> + svn_checksum_md5,
> >> + pool), pool);
> >>
> >> if (actual_checksum && (strcmp(text_checksum, actual_checksum) != 0))
> >> {
> >>
> >> Modified: trunk/subversion/libsvn_delta/text_delta.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_delta/text_delta.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_delta/text_delta.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_delta/text_delta.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -359,7 +359,7 @@ txdelta_md5_digest(void *baton)
> >> return NULL;
> >>
> >> /* The checksum should be there. */
> >> - return b->checksum->digest;
> >> + return svn_checksum_get_digest(b->checksum);
> >> }
> >>
> >>
> >>
> >> Modified: trunk/subversion/libsvn_fs/fs-loader.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs/fs-loader.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs/fs-loader.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs/fs-loader.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -970,7 +970,7 @@ svn_fs_file_checksum(svn_checksum_t **ch
> >> {
> >> SVN_ERR(root->vtable->file_checksum(checksum, kind, root, path, pool));
> >>
> >> - if (force && (*checksum == NULL || (*checksum)->kind != kind))
> >> + if (force && (*checksum == NULL || svn_checksum_get_kind(*checksum) != kind))
> >> {
> >> svn_stream_t *contents, *checksum_contents;
> >>
> >> @@ -996,7 +996,7 @@ svn_fs_file_md5_checksum(unsigned char d
> >>
> >> SVN_ERR(svn_fs_file_checksum(&md5sum, svn_checksum_md5, root, path, TRUE,
> >> pool));
> >> - memcpy(digest, md5sum->digest, APR_MD5_DIGESTSIZE);
> >> + memcpy(digest, svn_checksum_get_digest(md5sum), APR_MD5_DIGESTSIZE);
> >>
> >> return SVN_NO_ERROR;
> >> }
> >>
> >> Modified: trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_base/bdb/checksum-reps-table.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -77,7 +77,7 @@ svn_error_t *svn_fs_bdb__get_checksum_re
> >> int db_err;
> >>
> >> /* We only allow SHA1 checksums in this table. */
> >> - if (checksum->kind != svn_checksum_sha1)
> >> + if (svn_checksum_get_kind(checksum) != svn_checksum_sha1)
> >> return svn_error_create(SVN_ERR_BAD_CHECKSUM_KIND, NULL,
> >> _("Only SHA1 checksums can be used as keys in the "
> >> "checksum-reps table.\n"));
> >> @@ -106,7 +106,7 @@ svn_error_t *svn_fs_bdb__set_checksum_re
> >> int db_err;
> >>
> >> /* We only allow SHA1 checksums in this table. */
> >> - if (checksum->kind != svn_checksum_sha1)
> >> + if (svn_checksum_get_kind(checksum) != svn_checksum_sha1)
> >> return svn_error_create(SVN_ERR_BAD_CHECKSUM_KIND, NULL,
> >> _("Only SHA1 checksums can be used as keys in the "
> >> "checksum-reps table.\n"));
> >> @@ -149,7 +149,7 @@ svn_error_t *svn_fs_bdb__delete_checksum
> >> DBT key;
> >>
> >> /* We only allow SHA1 checksums in this table. */
> >> - if (checksum->kind != svn_checksum_sha1)
> >> + if (svn_checksum_get_kind(checksum) != svn_checksum_sha1)
> >> return svn_error_create(SVN_ERR_BAD_CHECKSUM_KIND, NULL,
> >> _("Only SHA1 checksums can be used as keys in the "
> >> "checksum-reps table.\n"));
> >>
> >> Modified: trunk/subversion/libsvn_fs_base/bdb/dbt.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/bdb/dbt.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_base/bdb/dbt.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_base/bdb/dbt.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -159,7 +159,8 @@ svn_fs_base__str_to_dbt(DBT *dbt, const
> >> DBT *
> >> svn_fs_base__checksum_to_dbt(DBT *dbt, svn_checksum_t *checksum)
> >> {
> >> - svn_fs_base__set_dbt(dbt, checksum->digest, svn_checksum_size(checksum));
> >> + svn_fs_base__set_dbt(dbt, svn_checksum_get_digest(checksum),
> >> + svn_checksum_get_size(checksum));
> >>
> >> return dbt;
> >> }
> >>
> >> Modified: trunk/subversion/libsvn_fs_base/dag.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/dag.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_base/dag.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_base/dag.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -1267,9 +1267,9 @@ svn_fs_base__dag_finalize_edits(dag_node
> >> {
> >> svn_checksum_t *test_checksum;
> >>
> >> - if (checksum->kind == svn_checksum_md5)
> >> + if (svn_checksum_get_kind(checksum) == svn_checksum_md5)
> >> test_checksum = md5_checksum;
> >> - else if (checksum->kind == svn_checksum_sha1)
> >> + else if (svn_checksum_get_kind(checksum) == svn_checksum_sha1)
> >> test_checksum = sha1_checksum;
> >> else
> >> return svn_error_create(SVN_ERR_BAD_CHECKSUM_KIND, NULL, NULL);
> >>
> >> Modified: trunk/subversion/libsvn_fs_base/reps-strings.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/reps-strings.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_base/reps-strings.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_base/reps-strings.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -816,8 +816,8 @@ svn_fs_base__rep_contents(svn_string_t *
> >>
> >> SVN_ERR(svn_fs_bdb__read_rep(&rep, fs, rep_key, trail, pool));
> >> rep_checksum = rep->sha1_checksum ? rep->sha1_checksum : rep->md5_checksum;
> >> - SVN_ERR(svn_checksum(&checksum, rep_checksum->kind, str->data, str->len,
> >> - pool));
> >> + SVN_ERR(svn_checksum(&checksum, svn_checksum_get_kind(rep_checksum),
> >> + str->data, str->len, pool));
> >>
> >> if (! svn_checksum_match(checksum, rep_checksum))
> >> return svn_error_createf
> >>
> >> Modified: trunk/subversion/libsvn_fs_base/tree.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/tree.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_base/tree.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_base/tree.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -3692,15 +3692,16 @@ txn_body_apply_textdelta(void *baton, tr
> >>
> >> /* Until we finalize the node, its data_key points to the old
> >> contents, in other words, the base text. */
> >> - SVN_ERR(svn_fs_base__dag_file_checksum(&checksum,
> >> - tb->base_checksum->kind,
> >> + SVN_ERR(svn_fs_base__dag_file_checksum(&checksum, svn_checksum_get_kind(
> >> + tb->base_checksum),
> >> tb->node, trail, trail->pool));
> >> /* 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))
> >> + if (svn_checksum_get_kind(tb->base_checksum)
> >> + == svn_checksum_get_kind(checksum)
> >> + && !svn_checksum_match(tb->base_checksum, checksum))
> >> return svn_error_createf
> >> (SVN_ERR_CHECKSUM_MISMATCH,
> >> NULL,
> >>
> >> Modified: trunk/subversion/libsvn_fs_base/util/fs_skels.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_base/util/fs_skels.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_base/util/fs_skels.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_base/util/fs_skels.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -554,18 +554,18 @@ svn_fs_base__parse_representation_skel(r
> >> {
> >> svn_skel_t *checksum_skel = header_skel->children->next->next;
> >> rep->md5_checksum =
> >> - svn_checksum__from_digest((const unsigned char *)
> >> - (checksum_skel->children->next->data),
> >> - svn_checksum_md5, pool);
> >> + svn_checksum_from_digest((const unsigned char *)
> >> + (checksum_skel->children->next->data),
> >> + svn_checksum_md5, pool);
> >>
> >> /* SHA1 */
> >> if (header_skel->children->next->next->next)
> >> {
> >> checksum_skel = header_skel->children->next->next->next;
> >> rep->sha1_checksum =
> >> - svn_checksum__from_digest((const unsigned char *)
> >> - (checksum_skel->children->next->data),
> >> - svn_checksum_sha1, pool);
> >> + svn_checksum_from_digest((const unsigned char *)
> >> + (checksum_skel->children->next->data),
> >> + svn_checksum_sha1, pool);
> >> }
> >> }
> >>
> >> @@ -1102,17 +1102,17 @@ prepend_checksum(svn_skel_t *skel,
> >> {
> >> svn_skel_t *checksum_skel = svn_skel__make_empty_list(pool);
> >>
> >> - switch (checksum->kind)
> >> + switch (svn_checksum_get_kind(checksum))
> >> {
> >> case svn_checksum_md5:
> >> - svn_skel__prepend(svn_skel__mem_atom(checksum->digest,
> >> + svn_skel__prepend(svn_skel__mem_atom(svn_checksum_get_digest(checksum),
> >> APR_MD5_DIGESTSIZE, pool),
> >> checksum_skel);
> >> svn_skel__prepend(svn_skel__str_atom("md5", pool), checksum_skel);
> >> break;
> >>
> >> case svn_checksum_sha1:
> >> - svn_skel__prepend(svn_skel__mem_atom(checksum->digest,
> >> + svn_skel__prepend(svn_skel__mem_atom(svn_checksum_get_digest(checksum),
> >> APR_SHA1_DIGESTSIZE, pool),
> >> checksum_skel);
> >> svn_skel__prepend(svn_skel__str_atom("sha1", pool), checksum_skel);
> >>
> >> Modified: trunk/subversion/libsvn_fs_fs/dag.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_fs/dag.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_fs/dag.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_fs/dag.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -1013,7 +1013,8 @@ svn_fs_fs__dag_finalize_edits(dag_node_t
> >> svn_checksum_t *file_checksum;
> >>
> >> SVN_ERR(svn_fs_fs__dag_file_checksum(&file_checksum, file,
> >> - checksum->kind, pool));
> >> + svn_checksum_get_kind(checksum),
> >> + pool));
> >> if (!svn_checksum_match(checksum, file_checksum))
> >> return svn_error_createf(SVN_ERR_CHECKSUM_MISMATCH, NULL,
> >> _("Checksum mismatch, file '%s':\n"
> >>
> >> Modified: trunk/subversion/libsvn_fs_fs/fs_fs.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_fs/fs_fs.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -3234,8 +3234,8 @@ delta_read_md5_digest(void *baton)
> >> {
> >> struct delta_read_baton *drb = baton;
> >>
> >> - if (drb->checksum->kind == svn_checksum_md5)
> >> - return drb->checksum->digest;
> >> + if (svn_checksum_get_kind(drb->checksum) == svn_checksum_md5)
> >> + return svn_checksum_get_digest(drb->checksum);
> >> else
> >> return NULL;
> >> }
> >>
> >> Modified: trunk/subversion/libsvn_fs_fs/rep-cache.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_fs/rep-cache.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_fs/rep-cache.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_fs/rep-cache.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -114,7 +114,7 @@ svn_fs_fs__get_rep_reference(representat
> >> }
> >>
> >> /* We only allow SHA1 checksums in this table. */
> >> - if (checksum->kind != svn_checksum_sha1)
> >> + if (svn_checksum_get_kind(checksum) != svn_checksum_sha1)
> >> return svn_error_create(SVN_ERR_BAD_CHECKSUM_KIND, NULL,
> >> _("Only SHA1 checksums can be used as keys in the "
> >> "rep_cache table.\n"));
> >>
> >> Modified: trunk/subversion/libsvn_fs_fs/tree.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_fs_fs/tree.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_fs_fs/tree.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_fs_fs/tree.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -2434,7 +2434,9 @@ apply_textdelta(void *baton, apr_pool_t
> >> /* Until we finalize the node, its data_key points to the old
> >> contents, in other words, the base text. */
> >> SVN_ERR(svn_fs_fs__dag_file_checksum(&checksum, tb->node,
> >> - tb->base_checksum->kind, pool));
> >> + svn_checksum_get_kind(
> >> + tb->base_checksum),
> >> + pool));
> >> if (!svn_checksum_match(tb->base_checksum, checksum))
> >> return svn_error_createf
> >> (SVN_ERR_CHECKSUM_MISMATCH,
> >>
> >> Modified: trunk/subversion/libsvn_subr/checksum.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/checksum.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_subr/checksum.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_subr/checksum.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -35,6 +35,16 @@
> >> #define DIGESTSIZE(k) ((k) == svn_checksum_md5 ? APR_MD5_DIGESTSIZE : \
> >> (k) == svn_checksum_sha1 ? APR_SHA1_DIGESTSIZE : 0)
> >>
> >> +struct svn_checksum_t
> >> +{
> >> + /** The bytes of the checksum digest. */
> >> + unsigned char *digest;
> >> +
> >> + /** The type of the checksum. This should never be changed by consumers
> >> + of the APIs. */
> >> + svn_checksum_kind_t kind;
> >> +};
> >> +
> >>
> >> /* Check to see if KIND is something we recognize. If not, return
> >> * SVN_ERR_BAD_CHECKSUM_KIND */
> >> @@ -69,13 +79,13 @@ svn_checksum_create(svn_checksum_kind_t
> >> }
> >>
> >> svn_checksum_t *
> >> -svn_checksum__from_digest(const unsigned char *digest,
> >> - svn_checksum_kind_t kind,
> >> - apr_pool_t *result_pool)
> >> +svn_checksum_from_digest(const unsigned char *digest,
> >> + svn_checksum_kind_t kind,
> >> + apr_pool_t *result_pool)
> >> {
> >> svn_checksum_t *checksum = svn_checksum_create(kind, result_pool);
> >>
> >> - memcpy((unsigned char *)checksum->digest, digest, DIGESTSIZE(kind));
> >> + memcpy(checksum->digest, digest, DIGESTSIZE(kind));
> >> return checksum;
> >> }
> >>
> >> @@ -84,7 +94,7 @@ svn_checksum_clear(svn_checksum_t *check
> >> {
> >> SVN_ERR(validate_kind(checksum->kind));
> >>
> >> - memset((unsigned char *) checksum->digest, 0, DIGESTSIZE(checksum->kind));
> >> + memset(checksum->digest, 0, DIGESTSIZE(checksum->kind));
> >> return SVN_NO_ERROR;
> >> }
> >>
> >> @@ -168,7 +178,7 @@ svn_checksum_parse_hex(svn_checksum_t **
> >> if ((! isxdigit(hex[i * 2])) || (! isxdigit(hex[i * 2 + 1])))
> >> return svn_error_create(SVN_ERR_BAD_CHECKSUM_PARSE, NULL, NULL);
> >>
> >> - ((unsigned char *)(*checksum)->digest)[i] =
> >> + (*checksum)->digest[i] =
> >> (( isalpha(hex[i*2]) ? hex[i*2] - 'a' + 10 : hex[i*2] - '0') << 4) |
> >> ( isalpha(hex[i*2+1]) ? hex[i*2+1] - 'a' + 10 : hex[i*2+1] - '0');
> >> is_zeros |= (*checksum)->digest[i];
> >> @@ -223,13 +233,13 @@ svn_checksum(svn_checksum_t **checksum,
> >> switch (kind)
> >> {
> >> case svn_checksum_md5:
> >> - apr_md5((unsigned char *)(*checksum)->digest, data, len);
> >> + apr_md5((*checksum)->digest, data, len);
> >> break;
> >>
> >> case svn_checksum_sha1:
> >> apr_sha1_init(&sha1_ctx);
> >> apr_sha1_update(&sha1_ctx, data, len);
> >> - apr_sha1_final((unsigned char *)(*checksum)->digest, &sha1_ctx);
> >> + apr_sha1_final((*checksum)->digest, &sha1_ctx);
> >> break;
> >>
> >> default:
> >> @@ -250,12 +260,12 @@ svn_checksum_empty_checksum(svn_checksum
> >> switch (kind)
> >> {
> >> case svn_checksum_md5:
> >> - memcpy((unsigned char *)checksum->digest, svn_md5__empty_string_digest(),
> >> + memcpy(checksum->digest, svn_md5__empty_string_digest(),
> >> APR_MD5_DIGESTSIZE);
> >> break;
> >>
> >> case svn_checksum_sha1:
> >> - memcpy((unsigned char *)checksum->digest,
> >> + memcpy(checksum->digest,
> >> svn_sha1__empty_string_digest(), APR_SHA1_DIGESTSIZE);
> >> break;
> >>
> >> @@ -332,11 +342,11 @@ svn_checksum_final(svn_checksum_t **chec
> >> switch (ctx->kind)
> >> {
> >> case svn_checksum_md5:
> >> - apr_md5_final((unsigned char *)(*checksum)->digest, ctx->apr_ctx);
> >> + apr_md5_final((*checksum)->digest, ctx->apr_ctx);
> >> break;
> >>
> >> case svn_checksum_sha1:
> >> - apr_sha1_final((unsigned char *)(*checksum)->digest, ctx->apr_ctx);
> >> + apr_sha1_final((*checksum)->digest, ctx->apr_ctx);
> >> break;
> >>
> >> default:
> >> @@ -348,7 +358,19 @@ svn_checksum_final(svn_checksum_t **chec
> >> }
> >>
> >> apr_size_t
> >> -svn_checksum_size(svn_checksum_t *checksum)
> >> +svn_checksum_get_size(const svn_checksum_t *checksum)
> >> {
> >> return DIGESTSIZE(checksum->kind);
> >> }
> >> +
> >> +svn_checksum_kind_t
> >> +svn_checksum_get_kind(const svn_checksum_t *checksum)
> >> +{
> >> + return checksum->kind;
> >> +}
> >> +
> >> +const unsigned char *
> >> +svn_checksum_get_digest(const svn_checksum_t *checksum)
> >> +{
> >> + return checksum->digest;
> >> +}
> >>
> >> Modified: trunk/subversion/libsvn_subr/io.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/io.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_subr/io.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_subr/io.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -1134,7 +1134,7 @@ svn_io_file_checksum(unsigned char diges
> >> svn_checksum_t *checksum;
> >>
> >> SVN_ERR(svn_io_file_checksum2(&checksum, file, svn_checksum_md5, pool));
> >> - memcpy(digest, checksum->digest, APR_MD5_DIGESTSIZE);
> >> + memcpy(digest, svn_checksum_get_digest(checksum), APR_MD5_DIGESTSIZE);
> >>
> >> return SVN_NO_ERROR;
> >> }
> >>
> >> Modified: trunk/subversion/libsvn_subr/stream.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/stream.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_subr/stream.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_subr/stream.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -911,14 +911,16 @@ close_handler_md5(void *baton)
> >> if (btn->read_digest)
> >> {
> >> *btn->read_digest = apr_palloc(btn->pool, APR_MD5_DIGESTSIZE);
> >> - memcpy((unsigned char *) *btn->read_digest, btn->read_checksum->digest,
> >> + memcpy((unsigned char *) *btn->read_digest,
> >> + svn_checksum_get_digest(btn->read_checksum),
> >> APR_MD5_DIGESTSIZE);
> >> }
> >>
> >> if (btn->write_digest)
> >> {
> >> *btn->write_digest = apr_palloc(btn->pool, APR_MD5_DIGESTSIZE);
> >> - memcpy((unsigned char *) *btn->write_digest, btn->write_checksum->digest,
> >> + memcpy((unsigned char *) *btn->write_digest,
> >> + svn_checksum_get_digest(btn->write_checksum),
> >> APR_MD5_DIGESTSIZE);
> >> }
> >>
> >>
> >> Modified: trunk/subversion/libsvn_subr/svn_base64.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_subr/svn_base64.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_subr/svn_base64.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_subr/svn_base64.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -388,8 +388,8 @@ svn_base64_from_checksum(const svn_check
> >> int ingrouplen = 0, linelen = 0;
> >> checksum_str = svn_stringbuf_create("", pool);
> >>
> >> - encode_bytes(checksum_str, checksum->digest,
> >> - svn_checksum_size(checksum), ingroup, &ingrouplen,
> >> + encode_bytes(checksum_str, svn_checksum_get_digest(checksum),
> >> + svn_checksum_get_size(checksum), ingroup, &ingrouplen,
> >> &linelen, TRUE);
> >> encode_partial_group(checksum_str, ingroup, ingrouplen, linelen, TRUE);
> >>
> >> @@ -410,8 +410,7 @@ svn_base64_from_md5(unsigned char digest
> >> {
> >> svn_checksum_t *checksum;
> >>
> >> - checksum = svn_checksum_create(svn_checksum_md5, pool);
> >> - checksum->digest = digest;
> >> + checksum = svn_checksum_from_digest(digest, svn_checksum_md5, pool);
> >>
> >> return svn_base64_from_checksum(checksum, pool);
> >> }
> >>
> >> Modified: trunk/subversion/libsvn_wc/adm_crawler.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/adm_crawler.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_wc/adm_crawler.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_wc/adm_crawler.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -971,7 +971,8 @@ svn_wc_transmit_text_deltas2(const char
> >> svn_path_local_style(path, pool)));
> >>
> >> if (digest)
> >> - memcpy(digest, local_checksum->digest, svn_checksum_size(local_checksum));
> >> + memcpy(digest, svn_checksum_get_digest(local_checksum),
> >> + svn_checksum_get_size(local_checksum));
> >>
> >> /* Close the file baton, and get outta here. */
> >> return editor->close_file
> >>
> >> Modified: trunk/subversion/libsvn_wc/adm_ops.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/adm_ops.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_wc/adm_ops.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_wc/adm_ops.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -678,8 +678,8 @@ svn_wc_queue_committed(svn_wc_committed_
> >> svn_checksum_t *checksum;
> >>
> >> if (digest)
> >> - checksum = svn_checksum__from_digest(digest, svn_checksum_md5,
> >> - (*queue)->pool);
> >> + checksum = svn_checksum_from_digest(digest, svn_checksum_md5,
> >> + (*queue)->pool);
> >> else
> >> checksum = NULL;
> >>
> >> @@ -817,7 +817,7 @@ svn_wc_process_committed4(const char *pa
> >> int log_number = 0;
> >>
> >> if (digest)
> >> - checksum = svn_checksum__from_digest(digest, svn_checksum_md5, pool);
> >> + checksum = svn_checksum_from_digest(digest, svn_checksum_md5, pool);
> >> else
> >> checksum = NULL;
> >>
> >>
> >> Modified: trunk/subversion/libsvn_wc/update_editor.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/update_editor.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/libsvn_wc/update_editor.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/libsvn_wc/update_editor.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -936,7 +936,7 @@ window_handler(svn_txdelta_window_t *win
> >>
> >> /* ... and its checksum. */
> >> fb->actual_checksum =
> >> - svn_checksum__from_digest(hb->digest, svn_checksum_md5, fb->pool);
> >> + svn_checksum_from_digest(hb->digest, svn_checksum_md5, fb->pool);
> >> }
> >>
> >> svn_pool_destroy(hb->pool);
> >>
> >> Modified: trunk/subversion/tests/libsvn_delta/window-test.c
> >> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/tests/libsvn_delta/window-test.c?pathrev=35103&r1=35102&r2=35103
> >> ==============================================================================
> >> --- trunk/subversion/tests/libsvn_delta/window-test.c Fri Jan 9 00:22:40 2009 (r35102)
> >> +++ trunk/subversion/tests/libsvn_delta/window-test.c Fri Jan 9 02:04:35 2009 (r35103)
> >> @@ -84,8 +84,8 @@ stream_window_test(const char **msg,
> >> /* ### examine the window */
> >> }
> >>
> >> - actual = svn_checksum_create(svn_checksum_md5, pool);
> >> - actual->digest = svn_txdelta_md5_digest(txstream);
> >> + actual = svn_checksum_from_digest(svn_txdelta_md5_digest(txstream),
> >> + svn_checksum_md5, pool);
> >> printf(" actual: %s\n", svn_checksum_to_cstring(actual, pool));
> >>
> >> if (!svn_checksum_match(expected, actual))
> >>
> >> ------------------------------------------------------
> >> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1013496
> >>
> >
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1013679
Received on 2009-01-09 13:44:33 CET