> -----Original Message-----
> From: kameshj tigris.org [mailto:kameshjtigris.org]
> Sent: Thursday, September 27, 2007 10:25 AM
> To: svnsubversion.tigris.org
> Subject: svn commit: r26803 - in trunk/subversion:
> libsvn_client tests/cmdline
>
> Author: kameshj
> Date: Thu Sep 27 07:25:29 2007
> New Revision: 26803
>
> Log:
> Fix issue 2821.
> Improve merge algorithm used for target trees with
child
> paths which have
> mergeinfo that differs from their parent.
<snip>
> --- trunk/subversion/libsvn_client/merge.c (original)
> +++ trunk/subversion/libsvn_client/merge.c Thu Sep 27
07:25:29 2007
<snip>
> -3566,9 +3732,9
>
&initial_rev2,
> ctx, pool));
>
> - same_urls = (strcmp(URL1, URL2) == 0);
> + notify_b.same_urls = (strcmp(URL1, URL2) == 0);
>
> - if (!same_urls && record_only)
> + if (!notify_b.same_urls && record_only)
> return svn_error_create(SVN_ERR_INCORRECT_PARAMS,
NULL,
> _("Use of two URLs is
not
> compatible with "
> "mergeinfo
modification"));
> -3612,7 +3778,7
> recursive diff-editor thing. */
> else if (entry->kind == svn_node_dir)
> {
> - if (same_urls)
> + if (notify_b.same_urls)
> {
> /* Merge children with differing mergeinfo.
*/
>
>
SVN_ERR(discover_and_merge_children(&children_with_merge
info,
> -3625,26 +3791,27
> depth,
>
ignore_ancestry,
>
adm_access,
> +
¬ify_b,
>
&merge_cmd_baton,
> pool));
> }
> -
> - /* Merge of the actual target.*/
> - SVN_ERR(do_merge(URL1,
> - range.start,
> - URL2,
> - range.end,
> - is_rollback,
> -
merge_cmd_baton.target_missing_child,
> - target_wcpath,
> - adm_access,
> - depth,
> - ignore_ancestry,
> - &merge_callbacks,
> - &merge_cmd_baton,
> - children_with_mergeinfo,
> -
merge_cmd_baton.existing_mergeinfo ? 0 : -1,
> - pool));
> + else
> + {
> + SVN_ERR(do_merge(URL1,
> + range.start,
> + URL2,
> + range.end,
> +
merge_cmd_baton.target_missing_child,
> + target_wcpath,
> + adm_access,
> + depth,
> + ignore_ancestry,
> + &merge_callbacks,
> + ¬ify_b,
> + &merge_cmd_baton,
> + children_with_mergeinfo,
> + pool));
> + }
Hi Kamesh,
Your change to call either discover_and_merge_children or
do_merge
allowed my compiler to finally catch this:
c:svnsrc-trunksubversionlibsvn_clientmerge.c(3813) :
warning C4700:
uninitialized local variable 'children_with_mergeinfo' used
This will lead to a segfault in drive_merge_report_editor()
if
do_merge() is ever called. Our test suite never provokes a
situation
like this, but it's easy enough to replicate:
Copy some directory DIR to DIR_COPY and commit it at rN
Make a modification to DIR in rN+1
Move DIR to DIR_MOVED in rN+2
Make a modification to DIR_MOVED in rN+3
Merge -rN:N+3 DIR_MOVED DIR_COPY -- and boom!
Anyhow, in r26813 I added an intialization and also tweaked
drive_merge_report_editor() to check for a possible NULL
children_with_mergeinfo (not really necessary right now, but
you check
it elsewhere in that function, so in the name of safety and
consistency...
Paul
> SVN_ERR(cleanup_noop_merge(&merge_cmd_baton,
> children_with_mergeinfo,
> adm_access, pool));
------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org
|
Paul Burba wrote:
> ..
> This will lead to a segfault in
drive_merge_report_editor() if
> do_merge() is ever called. Our test suite never
provokes a situation
> like this, but it's easy enough to replicate:
>
> Copy some directory DIR to DIR_COPY and commit it at
rN
> Make a modification to DIR in rN+1
> Move DIR to DIR_MOVED in rN+2
> Make a modification to DIR_MOVED in rN+3
> Merge -rN:N+3 DIR_MOVED DIR_COPY -- and boom!
>
So our test suite lacks this test?
> Anyhow, in r26813 I added an intialization and also
tweaked
> drive_merge_report_editor() to check for a possible
NULL
> children_with_mergeinfo (not really necessary right
now, but you check
> it elsewhere in that function, so in the name of safety
and
> consistency...
>
Just wondering why you didn't add that test in r26813? If
you don't have
time, why not add an XFail-ing place holder explaining in
comments the
above scenario, then we can still add it later.
Lieven
------------------------------------------------------------
---------
To unsubscribe, e-mail: dev-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org
|