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

Re: svn commit: r924236 - /subversion/trunk/subversion/libsvn_client/copy.c

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Wed, 17 Mar 2010 12:55:59 +0000

philip_at_apache.org wrote:
>
> +struct repos_to_wc_copy_baton {
> + const apr_array_header_t *copy_pairs;
> + const char *top_dst_path;
> + svn_boolean_t ignore_externals;
> + svn_ra_session_t *ra_session;
> + svn_client_ctx_t *ctx;
> +};
> +
> +static svn_error_t *
> +repos_to_wc_copy_cb(void *baton,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool)

Hi Philip. I see that this static function needs very little in the way
of a doc string, as it is currently obvious, but please could you add at
least one that says

/* Implements XXX_t. */

where XXX_t is the function type it conforms to, or a reference to where
it is specified, to save the reader having to follow through the call
site(s) to find out. I love being able to instantly "jump to
definition" in my editor.

- Julian

> +{
> + struct repos_to_wc_copy_baton *b = baton;
> +
> + SVN_ERR(repos_to_wc_copy_locked(b->copy_pairs, b->top_dst_path,
> + b->ignore_externals, b->ra_session,
> + b->ctx, scratch_pool));
> +
> + return SVN_NO_ERROR;
> +}
> +
> static svn_error_t *
> repos_to_wc_copy(const apr_array_header_t *copy_pairs,
> svn_boolean_t make_parents,
> @@ -1667,8 +1689,8 @@ repos_to_wc_copy(const apr_array_header_
> svn_ra_session_t *ra_session;
> const char *top_src_url, *top_dst_path;
> apr_pool_t *iterpool = svn_pool_create(pool);
> - svn_error_t *err1, *err2;
> - const char *anchor_abspath;
> + const char *lock_abspath;
> + struct repos_to_wc_copy_baton baton;
> int i;
>
> /* Get the real path for the source, based upon its peg revision. */
> @@ -1696,8 +1718,16 @@ repos_to_wc_copy(const apr_array_header_
> }
>
> get_copy_pair_ancestors(copy_pairs, &top_src_url, &top_dst_path, NULL, pool);
> + lock_abspath = top_dst_path;
> if (copy_pairs->nelts == 1)
> - top_src_url = svn_uri_dirname(top_src_url, pool);
> + {
> + svn_node_kind_t kind;
> + top_src_url = svn_uri_dirname(top_src_url, pool);
> + SVN_ERR(svn_wc__node_get_kind(&kind, ctx->wc_ctx, top_dst_path, FALSE,
> + pool));
> + if (kind != svn_node_dir)
> + lock_abspath = svn_dirent_dirname(top_dst_path, pool);
> + }
>
> /* Open a repository session to the longest common src ancestor. We do not
> (yet) have a working copy, so we don't have a corresponding path and
> @@ -1774,13 +1804,16 @@ repos_to_wc_copy(const apr_array_header_
> }
> svn_pool_destroy(iterpool);
>
> - SVN_ERR(svn_wc__acquire_write_lock(&anchor_abspath, ctx->wc_ctx, top_dst_path,
> - pool, pool));
> - err1 = repos_to_wc_copy_locked(copy_pairs, top_dst_path, ignore_externals,
> - ra_session, ctx, pool);
> - err2 = svn_wc__release_write_lock(ctx->wc_ctx, anchor_abspath, pool);
> -
> - return svn_error_compose_create(err1, err2);
> + baton.copy_pairs = copy_pairs;
> + baton.top_dst_path = top_dst_path;
> + baton.ignore_externals = ignore_externals;
> + baton.ra_session = ra_session;
> + baton.ctx = ctx;
> +
> + SVN_ERR(svn_wc__call_with_write_lock(repos_to_wc_copy_cb, &baton,
> + ctx->wc_ctx, lock_abspath,
> + pool, pool));
> + return SVN_NO_ERROR;
> }
>
> #define NEED_REPOS_REVNUM(revision) \
>
>
Received on 2010-03-17 13:56:32 CET

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