[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r948250 - in /subversion/trunk/subversion: libsvn_client/merge.c libsvn_client/mergeinfo.h tests/cmdline/merge_tests.py

From: Paul Burba <ptburba_at_gmail.com>
Date: Thu, 27 May 2010 13:01:54 -0400

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

This is an archived mail posted to the Subversion Dev mailing list.