Sounds like you're conflating two things: construction, and opacity. I
don't see them as tied.
If you're trying to fix the construction of these objects, and making
_from_digest part of our *public* API, then that is one discussion.
Placing function calls in between users and the fields of the
structure? That's just obstruction, with little benefit, IMO. I see no
value in that aspect of this change.
Cheers,
-g
On Fri, Jan 9, 2009 at 04:44, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> 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
>> >>
>> >
>>
>
>
Received on 2009-01-09 13:58:49 CET