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

Re: svn commit r1156750 - More fixes for issue #3986 'svn_client_mergeinfo_log API is broken'.

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Thu, 20 Oct 2011 12:46:36 +0100

Hi Paul.

r1156750 added a chunk of code in
svn_client__get_repos_mergeinfo_catalog() that appears to be adding a
prefix to a mergeinfo catalog.  There is already a function
"svn_mergeinfo__add_prefix_to_catalog()" for doing that; could you use
it? Suggested patch below.

Also r1156750 changed the line "repos_mergeinfo = NULL;" to
"*mergeinfo_cat = NULL;" which I think is wrong because in that case
the following "if (repos_mergeinfo_cat == NULL)" may be testing an
uninitialized value (depending on the implementation of
svn_ra_get_mergeinfo() in the case when it returns an error). See in
the patch below.

[[[
Index: subversion/libsvn_client/mergeinfo.c
===================================================================
--- subversion/libsvn_client/mergeinfo.c (revision 1186733)
+++ subversion/libsvn_client/mergeinfo.c (working copy)
@@ -487,54 +487,42 @@ svn_client__get_repos_mergeinfo_catalog(
   if (err)
     {
       if (squelch_incapable && err->apr_err == SVN_ERR_UNSUPPORTED_FEATURE)
         {
           svn_error_clear(err);
           *mergeinfo_cat = NULL;
+ /* ### should say "repos_mergeinfo_cat = NULL" or add a
"return" here? */
         }
       else
         return svn_error_trace(err);
     }

   if (repos_mergeinfo_cat == NULL)
     {
       *mergeinfo_cat = NULL;
     }
   else
     {
- const char *repos_root;
       const char *session_url;
+ const char *session_relpath;

- SVN_ERR(svn_ra_get_repos_root2(ra_session, &repos_root, scratch_pool));
       SVN_ERR(svn_ra_get_session_url(ra_session, &session_url, scratch_pool));
+ SVN_ERR(svn_ra_get_path_relative_to_root(ra_session, &session_relpath,
+ session_url, scratch_pool));

- if (strcmp(repos_root, session_url) == 0)
+ if (session_relpath[0] == '\0')
         {
           *mergeinfo_cat = repos_mergeinfo_cat;
         }
       else
         {
- apr_hash_index_t *hi;
- svn_mergeinfo_catalog_t rekeyed_mergeinfo_cat =
- apr_hash_make(result_pool);
-
- for (hi = apr_hash_first(scratch_pool, repos_mergeinfo_cat);
- hi;
- hi = apr_hash_next(hi))
- {
- const char *path =
- svn_path_url_add_component2(session_url,
- svn__apr_hash_index_key(hi),
- scratch_pool);
- SVN_ERR(svn_ra_get_path_relative_to_root(ra_session, &path,
- path,
- result_pool));
- apr_hash_set(rekeyed_mergeinfo_cat, path, APR_HASH_KEY_STRING,
- svn__apr_hash_index_val(hi));
- }
- *mergeinfo_cat = rekeyed_mergeinfo_cat;
+ SVN_ERR(svn_mergeinfo__add_prefix_to_catalog(mergeinfo_cat,
+ repos_mergeinfo_cat,
+ session_relpath,
+ result_pool,
+ scratch_pool));
         }
     }
   return SVN_NO_ERROR;
 }

]]]

- Julian
Received on 2011-10-20 13:47:11 CEST

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