On Wed, May 26, 2010 at 12:29 PM, Philip Martin
<philip.martin_at_wandisco.com> wrote:
> pburba_at_apache.org writes:
>
>> Author: pburba
>> Date: Tue May 25 23:28:46 2010
>> New Revision: 948250
>
>> +/* A svn_log_entry_receiver_t callback for
>> + get_inoperative_immediate_children(). */
>> +static svn_error_t *
>> +log_find_operative_subtree_revs(void *baton,
>> + svn_log_entry_t *log_entry,
>> + apr_pool_t *pool)
>
> Any reason why this is pool rather than scratch_pool?
Hi Philip,
I was simply using the same argument name in the implementation as is
used in the declaration of svn_log_entry_receiver_t. The POOL
argument to svn_log_entry_receiver_t should essentially be a
scratch_pool/iterpool per the docstring:
* Use @a pool for temporary allocation. If the caller is iterating
* over log messages, invoking this receiver on each, we recommend the
* standard pool loop recipe: create a subpool, pass it as @a pool to
* each call, clear it after each iteration, destroy it after the loop
* is done. (For allocation that must last beyond the lifetime of a
* given receiver call, use a pool in @a baton.)
>> +{
>> + apr_hash_t *immediate_children = baton;
>> + apr_hash_index_t *hi, *hi2;
>> + svn_revnum_t revision;
>> +
>> + revision = log_entry->revision;
>
> revision is not used.
Fixed.
>> +
>> + for (hi = apr_hash_first(pool, log_entry->changed_paths2);
>> + hi;
>> + hi = apr_hash_next(hi))
>> + {
>> + const char *path = svn__apr_hash_index_key(hi);
>> + for (hi2 = apr_hash_first(pool, immediate_children);
>
> It's only a small allocation so I suppose we can get away without an
> iteration pool.
As mentioned above, the POOL passed to this svn_log_entry_receiver_t
is effectively a scratch pool/iterpool, so we wouldn't gain anything.
>> + hi2;
>> + hi2 = apr_hash_next(hi2))
>> + {
>> + const char *immediate_path = svn__apr_hash_index_val(hi2);
>> + if (svn_dirent_is_ancestor(immediate_path, path))
>> + {
>> + apr_hash_set(immediate_children, svn__apr_hash_index_key(hi2),
>> + APR_HASH_KEY_STRING, NULL);
>> + /* A path can't be the child of more than
>> + one immediate child of the merge target. */
>> + break;
>> + }
>> + }
>> + }
>> + return SVN_NO_ERROR;
>> +}
>
>> +static svn_error_t *
>> +get_inoperative_immediate_children(apr_hash_t **immediate_children,
>> + apr_array_header_t *children_with_mergeinfo,
>> + const char *merge_source_repos_abspath,
>> + svn_revnum_t oldest_rev,
>> + svn_revnum_t youngest_rev,
>> + const char *merge_target_abspath,
>> + svn_ra_session_t *ra_session,
>> + apr_pool_t *result_pool,
>> + apr_pool_t *scratch_pool)
>> +{
>> + int i;
>> + *immediate_children = apr_hash_make(result_pool);
>> +
>> + SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(oldest_rev));
>> + SVN_ERR_ASSERT(SVN_IS_VALID_REVNUM(youngest_rev));
>> + SVN_ERR_ASSERT(oldest_rev <= youngest_rev);
>> +
>> + /* Find all the children in CHILDREN_WITH_MERGEINFO with the
>> + immediate_child_dir flag set and store them in *IMMEDIATE_CHILDREN. */
>> + for (i = 0; i < children_with_mergeinfo->nelts; i++)
>> + {
>> + svn_client__merge_path_t *child =
>> + APR_ARRAY_IDX(children_with_mergeinfo, i, svn_client__merge_path_t *);
>> +
>> + if (child->immediate_child_dir)
>> + apr_hash_set(*immediate_children,
>> + apr_pstrdup(result_pool, child->abspath),
>> + APR_HASH_KEY_STRING,
>> + svn_uri_join(merge_source_repos_abspath,
>> + svn_dirent_is_child(merge_target_abspath,
>> + child->abspath,
>> + result_pool),
>
> Arguments to svn_uri_join don't need to be in result pool. Use
> scratch_pool or an iteration pool.
Added an iterpool.
http://svn.apache.org/viewvc?view=revision&revision=948910
Thanks,
Paul
Received on 2010-05-27 19:02:30 CEST