Woah. This is even crazier than what you fixed. *nobody* asks for the end
revision. In fact, the code would have crashed since END_REVISION was
always NULL. Thus, ->kind must have never been unspecified. But no matter,
as the param is useless. Also note that only one callsite asks for the
start, and only one asks for the URL.
Seems you could simplify further...
Cheers,
-g
On Nov 5, 2011 10:02 AM, <julianfoad_at_apache.org> wrote:
> Author: julianfoad
> Date: Sat Nov 5 14:02:07 2011
> New Revision: 1197961
>
> URL: http://svn.apache.org/viewvc?rev=1197961&view=rev
> Log:
> Simplify calling svn_client__repos_locations() by returning svn_revnum_t
> instead of svn_opt_revision_t and by allowing NULL for unwanted revision
> number
> outputs.
>
> * subversion/libsvn_client/client.h
> (svn_client__repos_locations): Change the revision number outputs to
> svn_revnum_t and make them optional.
>
> * subversion/libsvn_client/copy.c
> (repos_to_repos_copy, repos_to_wc_copy): Eliminate dummy variables used in
> calls to svn_client__repos_locations().
>
> * subversion/libsvn_client/diff.c
> (diff_prepare_repos_repos, diff_repos_wc): Same.
>
> * subversion/libsvn_client/info.c
> (same_resource_in_head): Same.
>
> * subversion/libsvn_client/merge.c
> (filter_self_referential_mergeinfo, calculate_remaining_ranges,
> normalize_merge_sources, calculate_left_hand_side): Same.
>
> * subversion/libsvn_client/ra.c
> (resolve_rev_and_url): Eliminate a call that redundantly 'resolved' the
> already-resolved revision number.
> (svn_client__repos_locations): Change the revision number outputs to
> svn_revnum_t and make them optional.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/client.h
> subversion/trunk/subversion/libsvn_client/copy.c
> subversion/trunk/subversion/libsvn_client/diff.c
> subversion/trunk/subversion/libsvn_client/info.c
> subversion/trunk/subversion/libsvn_client/merge.c
> subversion/trunk/subversion/libsvn_client/ra.c
>
> Modified: subversion/trunk/subversion/libsvn_client/client.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/client.h?rev=1197961&r1=1197960&r2=1197961&view=diff
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/client.h (original)
> +++ subversion/trunk/subversion/libsvn_client/client.h Sat Nov 5 14:02:07
> 2011
> @@ -105,6 +105,7 @@ svn_error_t *svn_client__get_copy_source
> specify the point(s) of interest (typically the revisions referred
> to as the "operative range" for a given operation) along that history.
>
> + START_REVISION and/or END_REVISION may be NULL if not wanted.
> END may be NULL or of kind svn_opt_revision_unspecified (in either case
> END_URL and END_REVISION are not touched by the function);
> START and REVISION may not.
> @@ -131,9 +132,9 @@ svn_error_t *svn_client__get_copy_source
> Use POOL for all allocations. */
> svn_error_t *
> svn_client__repos_locations(const char **start_url,
> - svn_opt_revision_t **start_revision,
> + svn_revnum_t *start_revision,
> const char **end_url,
> - svn_opt_revision_t **end_revision,
> + svn_revnum_t *end_revision,
> svn_ra_session_t *ra_session,
> const char *path,
> const svn_opt_revision_t *revision,
>
> Modified: subversion/trunk/subversion/libsvn_client/copy.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/copy.c?rev=1197961&r1=1197960&r2=1197961&view=diff
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/copy.c (original)
> +++ subversion/trunk/subversion/libsvn_client/copy.c Sat Nov 5 14:02:07
> 2011
> @@ -795,7 +795,6 @@ repos_to_repos_copy(const apr_array_head
> svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
>
> svn_client__copy_pair_t *);
> apr_hash_t *mergeinfo;
> - svn_opt_revision_t *src_rev;
>
> /* Are the source and destination URLs at or under REPOS_ROOT? */
> if (! (svn_uri__is_ancestor(repos_root, pair->src_abspath_or_url)
> @@ -816,7 +815,7 @@ repos_to_repos_copy(const apr_array_head
> /* Run the history function to get the source's URL in the
> operational revision. */
> SVN_ERR(svn_ra_reparent(ra_session, pair->src_abspath_or_url, pool));
> - SVN_ERR(svn_client__repos_locations(&pair->src_abspath_or_url,
> &src_rev,
> + SVN_ERR(svn_client__repos_locations(&pair->src_abspath_or_url, NULL,
> NULL, NULL,
> ra_session,
> pair->src_abspath_or_url,
> @@ -1826,11 +1825,10 @@ repos_to_wc_copy(const apr_array_header_
> svn_client__copy_pair_t *pair = APR_ARRAY_IDX(copy_pairs, i,
>
> svn_client__copy_pair_t *);
> const char *src;
> - svn_opt_revision_t *new_rev;
>
> svn_pool_clear(iterpool);
>
> - SVN_ERR(svn_client__repos_locations(&src, &new_rev, NULL, NULL,
> + SVN_ERR(svn_client__repos_locations(&src, NULL, NULL, NULL,
> NULL,
> pair->src_abspath_or_url,
> &pair->src_peg_revision,
>
> Modified: subversion/trunk/subversion/libsvn_client/diff.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=1197961&r1=1197960&r2=1197961&view=diff
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/diff.c (original)
> +++ subversion/trunk/subversion/libsvn_client/diff.c Sat Nov 5 14:02:07
> 2011
> @@ -1497,10 +1497,8 @@ diff_prepare_repos_repos(const char **ur
> actual URLs will be. */
> if (peg_revision->kind != svn_opt_revision_unspecified)
> {
> - svn_opt_revision_t *start_ignore, *end_ignore;
> -
> - SVN_ERR(svn_client__repos_locations(url1, &start_ignore,
> - url2, &end_ignore,
> + SVN_ERR(svn_client__repos_locations(url1, NULL,
> + url2, NULL,
> *ra_session,
> path2,
> peg_revision,
> @@ -1835,9 +1833,7 @@ diff_repos_wc(const char *path1,
> actual URLs will be. */
> if (peg_revision->kind != svn_opt_revision_unspecified)
> {
> - svn_opt_revision_t *start_ignore;
> -
> - SVN_ERR(svn_client__repos_locations(&url1, &start_ignore, NULL,
> NULL,
> + SVN_ERR(svn_client__repos_locations(&url1, NULL, NULL, NULL,
> NULL,
> path1,
> peg_revision,
>
> Modified: subversion/trunk/subversion/libsvn_client/info.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/info.c?rev=1197961&r1=1197960&r2=1197961&view=diff
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/info.c (original)
> +++ subversion/trunk/subversion/libsvn_client/info.c Sat Nov 5 14:02:07
> 2011
> @@ -187,14 +187,13 @@ same_resource_in_head(svn_boolean_t *sam
> {
> svn_error_t *err;
> svn_opt_revision_t start_rev, peg_rev;
> - svn_opt_revision_t *ignored_rev;
> const char *head_url;
>
> start_rev.kind = svn_opt_revision_head;
> peg_rev.kind = svn_opt_revision_number;
> peg_rev.value.number = rev;
>
> - err = svn_client__repos_locations(&head_url, &ignored_rev, NULL, NULL,
> + err = svn_client__repos_locations(&head_url, NULL, NULL, NULL,
> ra_session,
> url, &peg_rev,
> &start_rev, NULL,
>
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge.c?rev=1197961&r1=1197960&r2=1197961&view=diff
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Sat Nov 5 14:02:07
> 2011
> @@ -928,7 +928,6 @@ filter_self_referential_mergeinfo(apr_ar
> for (j = 0; j < rangelist->nelts; j++)
> {
> svn_error_t *err2;
> - svn_opt_revision_t *start_revision;
> const char *start_url;
> svn_opt_revision_t peg_rev, rev1_opt;
> svn_merge_range_t *range =
> @@ -949,10 +948,8 @@ filter_self_referential_mergeinfo(apr_ar
>
> /* Check if PATH_at_BASE_REVISION exists at
> RANGE->START on the same line of history. */
> - err2 = svn_client__repos_locations(&start_url,
> - &start_revision,
> - NULL,
> - NULL,
> + err2 = svn_client__repos_locations(&start_url, NULL,
> + NULL, NULL,
> ra_session,
> target_url,
> &peg_rev,
> @@ -4008,13 +4005,13 @@ calculate_remaining_ranges(svn_client__m
> from our own future return a helpful error. */
> svn_error_t *err;
> const char *start_url;
> - svn_opt_revision_t requested, pegrev, *start_revision;
> + svn_opt_revision_t requested, pegrev;
> requested.kind = svn_opt_revision_number;
> requested.value.number = child_base_revision;
> pegrev.kind = svn_opt_revision_number;
> pegrev.value.number = revision1;
>
> - err = svn_client__repos_locations(&start_url, &start_revision,
> + err = svn_client__repos_locations(&start_url, NULL,
> NULL, NULL, ra_session, url1,
> &pegrev, &requested,
> NULL, ctx, scratch_pool);
> @@ -6498,13 +6495,13 @@ normalize_merge_sources(apr_array_header
> if (peg_revnum < youngest_requested)
> {
> const char *start_url;
> - svn_opt_revision_t requested, pegrev, *start_revision;
> + svn_opt_revision_t requested, pegrev;
> requested.kind = svn_opt_revision_number;
> requested.value.number = youngest_requested;
> pegrev.kind = svn_opt_revision_number;
> pegrev.value.number = peg_revnum;
>
> - SVN_ERR(svn_client__repos_locations(&start_url, &start_revision,
> + SVN_ERR(svn_client__repos_locations(&start_url, NULL,
> NULL, NULL,
> ra_session, source_url,
> &pegrev, &requested,
> @@ -10566,7 +10563,6 @@ calculate_left_hand_side(const char **ur
> youngest_merged_rev, from TARGET_ABSPATH to the source. Set
> *URL_LEFT and *REV_LEFT to cover the youngest part of this range.
> */
> svn_opt_revision_t peg_revision, youngest_rev;
> - svn_opt_revision_t *start_revision;
> const char *youngest_url;
>
> peg_revision.kind = svn_opt_revision_number;
> @@ -10578,7 +10574,7 @@ calculate_left_hand_side(const char **ur
> *rev_left = youngest_rev.value.number;
>
> /* Get the URL of the target_url_at_youngest_merged_rev. */
> - SVN_ERR(svn_client__repos_locations(&youngest_url, &start_revision,
> + SVN_ERR(svn_client__repos_locations(&youngest_url, NULL,
> NULL, NULL, target_ra_session,
> target_url, &peg_revision,
> &youngest_rev, NULL,
>
> Modified: subversion/trunk/subversion/libsvn_client/ra.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/ra.c?rev=1197961&r1=1197960&r2=1197961&view=diff
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/ra.c (original)
> +++ subversion/trunk/subversion/libsvn_client/ra.c Sat Nov 5 14:02:07 2011
> @@ -429,7 +429,6 @@ resolve_rev_and_url(svn_revnum_t *rev_p,
> {
> svn_opt_revision_t peg_rev = *peg_revision;
> svn_opt_revision_t start_rev = *revision;
> - svn_opt_revision_t *good_rev;
> const char *url;
> svn_revnum_t rev;
>
> @@ -440,15 +439,10 @@ resolve_rev_and_url(svn_revnum_t *rev_p,
>
> /* Run the history function to get the object's (possibly
> different) url in REVISION. */
> - SVN_ERR(svn_client__repos_locations(&url, &good_rev, NULL, NULL,
> + SVN_ERR(svn_client__repos_locations(&url, &rev, NULL, NULL,
> ra_session, path_or_url, &peg_rev,
> &start_rev, NULL, ctx, pool));
>
> - /* Resolve good_rev into a real revnum. */
> - if (good_rev->kind == svn_opt_revision_unspecified)
> - good_rev->kind = svn_opt_revision_head;
> - SVN_ERR(svn_client__get_revision_number(&rev, NULL, ctx->wc_ctx, url,
> - ra_session, good_rev, pool));
> if (rev_p)
> *rev_p = rev;
> if (url_p)
> @@ -586,9 +580,9 @@ svn_client__repos_location_segments(apr_
>
> svn_error_t *
> svn_client__repos_locations(const char **start_url,
> - svn_opt_revision_t **start_revision,
> + svn_revnum_t *start_revision,
> const char **end_url,
> - svn_opt_revision_t **end_revision,
> + svn_revnum_t *end_revision,
> svn_ra_session_t *ra_session,
> const char *path,
> const svn_opt_revision_t *revision,
> @@ -708,14 +702,13 @@ svn_client__repos_locations(const char *
> ra_session, end, pool));
>
> /* Set the output revision variables. */
> - *start_revision = apr_pcalloc(pool, sizeof(**start_revision));
> - (*start_revision)->kind = svn_opt_revision_number;
> - (*start_revision)->value.number = start_revnum;
> - if (end->kind != svn_opt_revision_unspecified)
> + if (start_revision)
> + {
> + *start_revision = start_revnum;
> + }
> + if (end_revision && end->kind != svn_opt_revision_unspecified)
> {
> - *end_revision = apr_pcalloc(pool, sizeof(**end_revision));
> - (*end_revision)->kind = svn_opt_revision_number;
> - (*end_revision)->value.number = end_revnum;
> + *end_revision = end_revnum;
> }
>
> if (start_revnum == peg_revnum && end_revnum == peg_revnum)
>
>
>
Received on 2011-11-05 21:12:08 CET