Two things that are not really about your patch, but more about the code that it touches:
> -----Original Message-----
> From: julianfoad_at_apache.org [mailto:julianfoad_at_apache.org]
> Sent: maandag 21 november 2011 15:07
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1204505 - in
> /subversion/trunk/subversion/libsvn_client: merge.c mergeinfo.c
> mergeinfo.h
>
> Author: julianfoad
> Date: Mon Nov 21 14:07:15 2011
> New Revision: 1204505
>
> URL: http://svn.apache.org/viewvc?rev=1204505&view=rev
> Log:
> Factor out another mergeinfo function.
>
> * subversion/libsvn_client/mergeinfo.h,
> subversion/libsvn_client/mergeinfo.c
> (svn_client__record_wc_mergeinfo_catalog): New function, extracted ...
>
> * subversion/libsvn_client/merge.c:
> (merge_cousins_and_supplement_mergeinfo): ... from here.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/merge.c
> subversion/trunk/subversion/libsvn_client/mergeinfo.c
> subversion/trunk/subversion/libsvn_client/mergeinfo.h
>
<snip>
> Modified: subversion/trunk/subversion/libsvn_client/mergeinfo.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/m
> ergeinfo.c?rev=1204505&r1=1204504&r2=1204505&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_client/mergeinfo.c (original)
> +++ subversion/trunk/subversion/libsvn_client/mergeinfo.c Mon Nov 21
> 14:07:15 2011
> @@ -147,6 +147,49 @@ svn_client__record_wc_mergeinfo(const ch
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_client__record_wc_mergeinfo_catalog(apr_hash_t *result_catalog,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *scratch_pool)
> +{
> + apr_pool_t *iterpool = svn_pool_create(scratch_pool);
> +
> + if (apr_hash_count(result_catalog))
> + {
> + int i;
> + apr_array_header_t *sorted_cat =
> + svn_sort__hash(result_catalog, svn_sort_compare_items_as_paths,
> + scratch_pool);
We create a sorted array from the hash here, which is then walked once and then the sorted array is released.
Can't we just walk the hash in its current order instead of creating the array?
My guess would be that only the error message might change. The caller returns an error on any failing item anyway.
> + for (i = 0; i < sorted_cat->nelts; i++)
> + {
> + svn_sort__item_t elt = APR_ARRAY_IDX(sorted_cat, i,
> + svn_sort__item_t);
> + svn_error_t *err;
> +
> + svn_pool_clear(iterpool);
> + err = svn_client__record_wc_mergeinfo(elt.key, elt.value, TRUE,
> + ctx, iterpool);
> +
> + if (err && err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
> + {
> + /* PATH isn't just missing, it's not even versioned as far
> + as this working copy knows. But it was included in
> + MERGES, which means that the server knows about it.
> + Likely we don't have access to the source due to authz
> + restrictions. For now just clear the error and
> + continue... */
> + svn_error_clear(err);
^^^ I would guess this error condition doesn't happen as this function appears to return SVN_ERR_WC_PATH_NOT_FOUND in 1.7 when the node is not found.
> + }
> + else
> + {
> + SVN_ERR(err);
> + }
> + }
> + }
> + svn_pool_destroy(iterpool);
> + return SVN_NO_ERROR;
> +}
> +
> /*-----------------------------------------------------------------------*/
>
Bert
Received on 2011-11-21 19:21:58 CET