[PATCH] Fix segfault in repo->wc copy Re: copy repo->wc segmentation fault

alan.wood_at_clear.net.nz wrote:
> I have just traced a bug in the "svn copy URL_at_rev file" command.
> While following instructions in the svn-book to retrieve a deleted file
> svn copy svn://fred.ncs.local/ncs/truck/.../a.txt_at_211 a.txt
> <segmentation fault>
> This occurred on a Linux machine.
> I repeated the command from windows command line with the same result.
>
> tested on svn version 1.5.1(Linux and win32) and 1.5.x branch(win32)
>
> I have traced this through to libsvn_client: copy.c line 1455
>
> The return from svn_wc_add_repos_file2() is not checked with SVN_ERR().
>
> This fails when the repository name does not exactly match the
> repository name on the command line.
> In my case the Linux attempt was different as I used the machines
> actual name "fred" as opposed to "subversion.ncs.local" which the
> working copy was built from. On the win32 case I was using a
> file:///c:/... on the command line, but the working copy was checked
> out by TortoiseSVN and had file:///C:/... in the working copy.
>
> The segmentation fault actually occurs a few lines down when calling
> extend_wc_mergeinfo() with dst_entry==NULL.
>
> After changing to catch the error from the svn_wc_add_repos_file2()
> I do get an error message about the repositories having different
> roots which with careful inspection of C vs c may help the user to
> get it right next time. Would be nicer to to check the repository by
> uuid rather than name, then check the path within the repository as
> a string. Or maybe I missed something completely.

I haven't thoroughly tested this, but figured I'd throw it out here. Alan, if
you could check and see if this fixes your problem, that'd be great. The patch
is against trunk, but should apply cleanly against 1.5.x.

I do wonder why we don't just wrap the call to svn_wc_add_repos_file2 with
SVN_ERR(). Is there a particular reason why we need to wait for timestamps
before returning the error?

-Hyrum

[[[
If we can't add the target of the copy to the working copy, don't continue
trying to calculate mergeinfo based upon the non-existent path. This fixes a
segfault reported here:
http://svn.haxx.se/dev/archive-2008-08/0667.shtml

Found by: Alan Wood <alan.wood_at_clear.net.nz>

* subversion/libsvn_client/copy.c
  (repos_to_wc_copy_single): Don't continue with mergeinfo calculation if the
   target of the copy couldn't be added to the working copy.
]]]

Index: subversion/libsvn_client/copy.c
===================================================================
--- subversion/libsvn_client/copy.c (revision 32740)
+++ subversion/libsvn_client/copy.c (working copy)
@@ -1472,24 +1472,27 @@
          same_repositories ? src_revnum : SVN_INVALID_REVNUM,
          pool);
 
- SVN_ERR(svn_wc_entry(&dst_entry, pair->dst, adm_access, FALSE, pool));
- SVN_ERR(calculate_target_mergeinfo(ra_session, &src_mergeinfo,
- NULL, pair->src, src_revnum,
- FALSE, ctx, pool));
- SVN_ERR(extend_wc_mergeinfo(pair->dst, dst_entry, src_mergeinfo,
- adm_access, ctx, pool));
+ if (!err)
+ {
+ SVN_ERR(svn_wc_entry(&dst_entry, pair->dst, adm_access, FALSE, pool));
+ SVN_ERR(calculate_target_mergeinfo(ra_session, &src_mergeinfo,
+ NULL, pair->src, src_revnum,
+ FALSE, ctx, pool));
+ SVN_ERR(extend_wc_mergeinfo(pair->dst, dst_entry, src_mergeinfo,
+ adm_access, ctx, pool));
 
- /* Ideally, svn_wc_add_repos_file() would take a notify function
- and baton, and we wouldn't have to make this call here.
- However, the situation is... complicated. See issue #1552
- for the full story. */
- if (!err && ctx->notify_func2)
- {
- svn_wc_notify_t *notify = svn_wc_create_notify(pair->dst,
- svn_wc_notify_add,
- pool);
- notify->kind = pair->src_kind;
- (*ctx->notify_func2)(ctx->notify_baton2, notify, pool);
+ /* Ideally, svn_wc_add_repos_file() would take a notify function
+ and baton, and we wouldn't have to make this call here.
+ However, the situation is... complicated. See issue #1552
+ for the full story. */
+ if (ctx->notify_func2)
+ {
+ svn_wc_notify_t *notify = svn_wc_create_notify(pair->dst,
+ svn_wc_notify_add,
+ pool);
+ notify->kind = pair->src_kind;
+ (*ctx->notify_func2)(ctx->notify_baton2, notify, pool);
+ }
         }
 
       svn_sleep_for_timestamps();