Matt Kraai <kraai@alumni.carnegiemellon.edu> writes:
> The following patch addresses one part of issue #406, the
> svn_fs_{node,txn,revision}_proplist functions. Earlier, I tried
> doing the whole thing in one patch but it was unmanageable. The
> following patch passed `make check', though the python tests were
> skipped as I don't have a sufficiently recent version installed.
I hate to do this to you, but the python tests *are* the test suite,
to a first approximation. If you didn't run them, then no significant
automated testing happened.
I just upgraded Python from source (www.python.org) at home yesterday
for this, it was a breeze. Strongly recommended... :-)
Can you do that and let us know if the full test suite passes with
this patch (or post a new patch if not)? Thanks,
-K
> * subversion/include/svn_string.h
>
> (svn_string_compare_stringbuf): New.
>
> * subversion/include/svn_fs.h
>
> (svn_fs_node_proplist, svn_fs_txn_proplist, svn_fs_revision_proplist):
> Note that the hash table contains pointers to svn_string_t objects.
>
> * subversion/libsvn_fs/this-branch.txt
>
> (svn_fs_node_proplist): Note that the hash table contains pointers to
> svn_string_t objects.
>
> * subversion/libsvn_fs/proplist.c
>
> (svn_fs__make_prop_hash): Insert svn_string_t * into the hash table.
>
> * subversion/libsvn_fs_proplist.h
>
> (svn_fs__make_prop_hash): Note that the hash table contains pointers
> to svn_string_t objects.
>
> * subversion/libsvn_subr/svn_string.c
>
> (svn_string_compare_stringbuf): New.
>
> * subversion/libsvn_ra_local/checkout.c
>
> (set_any_props): Construct svn_stringbuf_t.
>
> * subversion/tests/libsvn_fs/fs-test.c
>
> (revision_props, transaction_props, node_props): Use svn_string_t.
>
> * subversion/libsvn_repos/delta.c
>
> (delta_proplists): Construct svn_stringbuf_t, use
> svn_string_compare_stringbuf, and remove unused svn_stringbuf_t.
>
> Index: ./subversion/include/svn_string.h
> ===================================================================
> --- ./subversion/include/.svn/text-base/svn_string.h Thu Oct 25 10:57:47 2001
> +++ ./subversion/include/svn_string.h Thu Oct 25 12:18:54 2001
> @@ -225,6 +225,10 @@
> chopped, so if no such CHAR in STR, chops nothing and returns 0. */
> apr_size_t svn_stringbuf_chop_back_to_char (svn_stringbuf_t *str, char ch);
>
> +/* Return TRUE iff STR1 and STR2 have identical length and data. */
> +svn_boolean_t svn_string_compare_stringbuf (const svn_string_t *str1,
> + const svn_stringbuf_t *str2);
> +
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */
> Index: ./subversion/include/svn_fs.h
> ===================================================================
> --- ./subversion/include/.svn/text-base/svn_fs.h Thu Oct 25 10:57:46 2001
> +++ ./subversion/include/svn_fs.h Thu Oct 25 12:02:38 2001
> @@ -528,7 +528,7 @@
>
> /* Set *TABLE_P to the entire property list of transaction TXN in
> filesystem FS, as an APR hash table allocated in POOL. The
> - resulting table maps property names to pointers to svn_stringbuf_t
> + resulting table maps property names to pointers to svn_string_t
> objects containing the property value. */
> svn_error_t *svn_fs_txn_proplist (apr_hash_t **table_p,
> svn_fs_txn_t *txn,
> @@ -728,7 +728,7 @@
>
> /* Set *TABLE_P to the entire property list of PATH in ROOT, as an APR
> hash table allocated in POOL. The resulting table maps property
> - names to pointers to svn_stringbuf_t objects containing the property
> + names to pointers to svn_string_t objects containing the property
> value. */
> svn_error_t *svn_fs_node_proplist (apr_hash_t **table_p,
> svn_fs_root_t *root,
> @@ -1127,10 +1127,8 @@
>
> /* Set *TABLE_P to the entire property list of revision REV in
> filesystem FS, as an APR hash table allocated in POOL. The table
> - maps char * property names to svn_stringbuf_t * values; the names
> - and values are allocated in POOL.
> -
> - ### todo (issue #406): map to svn_string_t, not svn_stringbuf_t. */
> + maps char * property names to svn_string_t * values; the names
> + and values are allocated in POOL. */
> svn_error_t *svn_fs_revision_proplist (apr_hash_t **table_p,
> svn_fs_t *fs,
> svn_revnum_t rev,
> Index: ./subversion/libsvn_fs/this-branch.txt
> ===================================================================
> --- ./subversion/libsvn_fs/.svn/text-base/this-branch.txt Thu Oct 25 10:57:34 2001
> +++ ./subversion/libsvn_fs/this-branch.txt Thu Oct 25 12:09:37 2001
> @@ -81,7 +81,7 @@
>
> /* Set *TABLE_P to the entire property list of PATH in ROOT, as an APR
> hash table allocated in POOL. The resulting table maps property
> - names to pointers to svn_stringbuf_t objects containing the property
> + names to pointers to svn_string_t objects containing the property
> value. */
> svn_error_t *svn_fs_node_proplist (apr_hash_t **table_p,
> svn_fs_root_t *root,
> Index: ./subversion/libsvn_fs/proplist.c
> ===================================================================
> --- ./subversion/libsvn_fs/.svn/text-base/proplist.c Thu Oct 25 10:57:33 2001
> +++ ./subversion/libsvn_fs/proplist.c Thu Oct 25 12:29:17 2001
> @@ -84,7 +84,7 @@
> apr_hash_set (table,
> apr_pstrndup(pool, this_name->data, this_name->len),
> this_name->len,
> - svn_stringbuf_ncreate (this_value->data,
> + svn_string_ncreate (this_value->data,
> this_value->len,
> pool));
> }
> Index: ./subversion/libsvn_fs/proplist.h
> ===================================================================
> --- ./subversion/libsvn_fs/.svn/text-base/proplist.h Thu Oct 25 10:57:32 2001
> +++ ./subversion/libsvn_fs/proplist.h Thu Oct 25 12:31:58 2001
> @@ -36,18 +36,9 @@
> const svn_string_t *name,
> apr_pool_t *pool);
>
> -/* Set *PROP_HASH to a hash table mapping char * names to
> +/* Set *PROP_HASH to a hash table mapping const char * names to
> svn_stringbuf_t * values, based on PROPLIST. The hash table and
> - its name/value pairs are all allocated in POOL.
> -
> - ### todo (issue #406): first of all, the hash should be mapping
> - names to svn_string_t's, not svn_stringbuf_t. Second, the fact
> - that the keys are char *'s is inconsistent with other interfaces by
> - which we set property names, which all take an svn_string_t or
> - svn_stringbuf_t right now (usually the former). Probably using
> - const char * is best -- I mean, who really wants binary property
> - names? -- but we need to be consistent about it. This change would
> - affect a lot of functions, not just here. */
> + its name/value pairs are all allocated in POOL. */
> svn_error_t *svn_fs__make_prop_hash (apr_hash_t **prop_hash,
> skel_t *proplist,
> apr_pool_t *pool);
> Index: ./subversion/libsvn_subr/svn_string.c
> ===================================================================
> --- ./subversion/libsvn_subr/.svn/text-base/svn_string.c Thu Oct 25 10:57:21 2001
> +++ ./subversion/libsvn_subr/svn_string.c Thu Oct 25 12:27:18 2001
> @@ -504,6 +504,23 @@
> }
>
>
> +svn_boolean_t
> +svn_string_compare_stringbuf (const svn_string_t *str1,
> + const svn_stringbuf_t *str2)
> +{
> + /* easy way out :) */
> + if (str1->len != str2->len)
> + return FALSE;
> +
> + /* now that we know they have identical lengths... */
> +
> + if (memcmp (str1->data, str2->data, str1->len))
> + return FALSE;
> + else
> + return TRUE;
> +}
> +
> +
>
> /* --------------------------------------------------------------
> * local variables:
> Index: ./subversion/libsvn_ra_local/checkout.c
> ===================================================================
> --- ./subversion/libsvn_ra_local/.svn/text-base/checkout.c Thu Oct 25 10:57:15 2001
> +++ ./subversion/libsvn_ra_local/checkout.c Thu Oct 25 12:08:36 2001
> @@ -93,8 +93,8 @@
> svn_stringbuf_t *name, *value;
>
> apr_hash_this (hi, &key, &klen, &val);
> - value = (svn_stringbuf_t *) val;
> name = svn_stringbuf_ncreate (key, klen, pool);
> + value = svn_stringbuf_create_from_string ((svn_string_t *) val, pool);
>
> if (is_dir)
> SVN_ERR (editor->change_dir_prop (object_baton, name, value));
> Index: ./subversion/tests/libsvn_fs/fs-test.c
> ===================================================================
> --- ./subversion/tests/libsvn_fs/.svn/text-base/fs-test.c Thu Oct 25 10:58:12 2001
> +++ ./subversion/tests/libsvn_fs/fs-test.c Thu Oct 25 12:22:55 2001
> @@ -737,7 +737,7 @@
> the expected values. */
> SVN_ERR (svn_fs_revision_proplist (&proplist, fs, 0, pool));
> {
> - svn_stringbuf_t *prop_value;
> + svn_string_t *prop_value;
>
> if (apr_hash_count (proplist) < 4 )
> return svn_error_createf
> @@ -850,7 +850,7 @@
> the expected values. */
> SVN_ERR (svn_fs_txn_proplist (&proplist, txn, pool));
> {
> - svn_stringbuf_t *prop_value;
> + svn_string_t *prop_value;
>
> /* All transactions get a datestamp property at their inception,
> so we expect *5*, not 4 properties. */
> @@ -900,7 +900,7 @@
> existed on the transaction just prior to its being committed. */
> SVN_ERR (svn_fs_revision_proplist (&proplist, fs, after_rev, pool));
> {
> - svn_stringbuf_t *prop_value;
> + svn_string_t *prop_value;
>
> if (apr_hash_count (proplist) < 5 )
> return svn_error_createf
> @@ -1018,7 +1018,7 @@
> the expected values. */
> SVN_ERR (svn_fs_node_proplist (&proplist, txn_root, "music.txt", pool));
> {
> - svn_stringbuf_t *prop_value;
> + svn_string_t *prop_value;
>
> if (apr_hash_count (proplist) != 4 )
> return svn_error_createf
> Index: ./subversion/libsvn_repos/delta.c
> ===================================================================
> --- ./subversion/libsvn_repos/.svn/text-base/delta.c Thu Oct 25 10:58:00 2001
> +++ ./subversion/libsvn_repos/delta.c Thu Oct 25 12:20:05 2001
> @@ -453,7 +453,8 @@
>
> for (hi = apr_hash_first (subpool, t_props); hi; hi = apr_hash_next (hi))
> {
> - svn_stringbuf_t *s_value, *t_value, *t_name;
> + svn_string_t *s_value;
> + svn_stringbuf_t *t_value, *t_name;
> const void *key;
> void *val;
> apr_size_t klen;
> @@ -461,7 +462,8 @@
> /* KEY is property name in target, VAL the value */
> apr_hash_this (hi, &key, &klen, &val);
> t_name = svn_stringbuf_ncreate (key, klen, subpool);
> - t_value = val;
> + t_value = svn_stringbuf_create_from_string ((svn_string_t *) val,
> + subpool);
>
> /* See if this property existed in the source. If so, and if
> the values in source and target differ, replace the value in
> @@ -469,7 +471,7 @@
> if (s_props
> && ((s_value = apr_hash_get (s_props, key, klen)) != 0))
> {
> - if (svn_stringbuf_compare (s_value, t_value))
> + if (svn_string_compare_stringbuf (s_value, t_value))
> SVN_ERR (change_fn (c, object, t_name, t_value, subpool));
>
> /* Remove the property from source list so we can track
> @@ -490,7 +492,7 @@
> {
> for (hi = apr_hash_first (subpool, s_props); hi; hi = apr_hash_next (hi))
> {
> - svn_stringbuf_t *s_value, *s_name;
> + svn_stringbuf_t *s_name;
> const void *key;
> void *val;
> apr_size_t klen;
> @@ -498,7 +500,6 @@
> /* KEY is property name in target, VAL the value */
> apr_hash_this (hi, &key, &klen, &val);
> s_name = svn_stringbuf_ncreate (key, klen, subpool);
> - s_value = val;
>
> SVN_ERR (change_fn (c, object, s_name, NULL, subpool));
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:45 2006