On 10/26/2012 11:34 AM, Philip Martin wrote:
> Philip Martin <philip.martin_at_wandisco.com> writes:
>
>>> ==15116== Source and destination overlap in memcpy(0x9e3f078, 0x98aca80, 160094512)
>>> ==15116== at 0x4C25F6A: memcpy (mc_replace_strmem.c:497)
>>> ==15116== by 0x5A25229: svn_string_ncreate (string.c:165)
>>> ==15116== by 0x5A2541B: svn_string_dup (string.c:224)
>>> ==15116== by 0x5A1650C: svn_prop_diffs (properties.c:225)
>>> ==15116== by 0x513DE67: set_props_txn (wc_db.c:5139)
>>> ==15116== by 0x515F337: run_txn (wc_db_util.c:188)
>>> ==15116== by 0x5A1F9FA: svn_sqlite__with_lock (sqlite.c:1073)
>>> ==15116== by 0x515F3AD: svn_wc__db_with_txn (wc_db_util.c:210)
>>> ==15116== by 0x513E17A: svn_wc__db_op_set_props (wc_db.c:5189)
>>> ==15116== by 0x510DB3C: do_propset (props.c:1941)
>>> ==15116== by 0x510E15D: svn_wc_prop_set4 (props.c:2073)
>>> ==15116== by 0x4E3AAB2: add_file (add.c:327)
>>> ==15116==
>>
>> I assume this is the problem and that copying overlapping memory is
>> causing corruption to the pools and/or the malloc structures:
>
> This makes the test PASS:
>
> Index: ../src/subversion/libsvn_client/add.c
> ===================================================================
> --- ../src/subversion/libsvn_client/add.c (revision 1401908)
> +++ ../src/subversion/libsvn_client/add.c (working copy)
> @@ -146,8 +146,8 @@
> const char *filename,
> const char *pattern,
> apr_hash_t *propvals,
> - apr_pool_t *scratch_pool,
> - apr_pool_t *result_pool)
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)
> {
> apr_hash_index_t *hi;
>
> @@ -163,7 +163,7 @@
> const char *propname = svn__apr_hash_index_key(hi);
> const char *propval = svn__apr_hash_index_val(hi);
> svn_string_t *propval_str = apr_palloc(result_pool,
> - sizeof(*propval));
> + sizeof(*propval_str));
> propval_str->data = propval;
> propval_str->len = strlen(propval);
>
Nice. I don't wonder, though, why we pass "result_pool" at all. Unless we
are specifically allowing for a scenario where the lifetime of the items
*in* the hash differs from that of the hash itself, I would drop the
"result_pool" parameter and use "apr_hash_pool_get(properties)" (or
whatever) instead within this helper function's body.
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Received on 2012-10-26 17:54:52 CEST