Re: [PATCH][MERGE-TRACKING] Step 2 of repos to repos copyfrom info recording

> +/* Log callback for obtaining the copyfrom revision of
> + a given path. */
> +static svn_error_t *
> +copyfrom_receiver(void *baton,
> + apr_hash_t *changed_paths,
> + svn_revnum_t revision,
> + const char *author,
> + const char *date,
> + const char *message,
> + apr_pool_t *pool)
> +{
> + svn_revnum_t *svn_revnum = baton;
>
> + *svn_revnum = revision;
>
I could see duplicate assignment here ???

> + return SVN_NO_ERROR;
> +}
> +
> +/* Obtain the copyfrom mergeinfo of the given path. */
> static svn_error_t *
> +get_copyfrom_mergeinfo(svn_ra_session_t *ra_session,
> + apr_hash_t **copyfrom_mergeinfo,
> + const char *rel_path,
> + const char *copyfrom_path,
> + svn_revnum_t src_revnum,
> + apr_pool_t *pool)
> +{
> + svn_revnum_t copyfrom_rev;
> + svn_merge_range_t *copyfrom_mergerange;
> + apr_array_header_t *copyfrom_mergerange_list;
> + apr_array_header_t *rel_paths = apr_array_make(pool, 1,
> + sizeof(const char *));
> +
> + *copyfrom_mergeinfo = NULL;
> +
> + APR_ARRAY_PUSH(rel_paths, const char *) = rel_path;
> +
> + /* Extract loginfo for finding the copyfrom rev */
> + SVN_ERR(svn_ra_get_log(ra_session, rel_paths, 1, src_revnum,
> + 1, FALSE, TRUE, copyfrom_receiver,
> + &copyfrom_rev, pool));
> +
> + copyfrom_mergerange = apr_palloc(pool, sizeof(*copyfrom_mergerange));
> + copyfrom_mergerange->start = copyfrom_rev;
> + copyfrom_mergerange->end = src_revnum;
> + copyfrom_mergerange_list = apr_array_make(pool, 1,
> + sizeof(copyfrom_mergerange));
> + APR_ARRAY_PUSH(copyfrom_mergerange_list, svn_merge_range_t *)
> + = copyfrom_mergerange;
> + *copyfrom_mergeinfo = apr_hash_make(pool);
> + apr_hash_set(*copyfrom_mergeinfo, copyfrom_path,
> + sizeof(copyfrom_mergerange_list), copyfrom_mergerange_list);
> +
>
Should it not be

apr_hash_set(*copyfrom_mergeinfo, copyfrom_path,
               APR_HASH_KEY_STRING, copyfrom_mergerange_list);

> + return SVN_NO_ERROR;
> +}
> +
> +/* Obtain the copyfrom mergeinfo and the existing mergeinfo of
> + the source path, merge them and set as a svn_string_t in
> + TARGET_MERGEINFO. */
> +static svn_error_t *
> +calculate_target_mergeinfo(svn_ra_session_t *ra_session,
> + svn_string_t **target_mergeinfo,
> + const char *src_url,
> + const char *src_rel,
> + svn_revnum_t src_revnum,
> + apr_pool_t *pool)
> +{
> + const char *repos_root;
> + const char *copyfrom_path = src_url;
> + apr_hash_t *copyfrom_mergeinfo;
> +
> + /* Find src path relative to the repos root */
> + SVN_ERR(svn_ra_get_repos_root(ra_session, &repos_root, pool));
> + while (*copyfrom_path++ == *repos_root++ );
> + copyfrom_path--;
>
I prefer preincrement as it does not need a temp copy of variable, may
be the compiler could be intelligent in making them preincrements in
void contexts like this.
Why not

while (*(--copyfrom_path) == *(--repos_root) );
  --copyfrom_path;

With regards
Kamesh Jayachandran

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Sep 27 13:10:41 2006