On Mon, Nov 21, 2011 at 12:12:56PM +0000, Philip Martin wrote:
> Philip Martin <philip.martin_at_wandisco.com> writes:
>
> > This hack fixes the problem:
> >
> > Index: subversion/libsvn_client/merge.c
> > ===================================================================
> > --- subversion/libsvn_client/merge.c (revision 1203766)
> > +++ subversion/libsvn_client/merge.c (working copy)
> > @@ -593,6 +593,7 @@ tree_conflict(merge_cmd_baton_t *merge_b,
> > if (merge_b->conflicted_paths == NULL)
> > merge_b->conflicted_paths = apr_hash_make(merge_b->pool);
> >
> > + victim_abspath = apr_pstrdup(merge_b->pool, victim_abspath);
> > apr_hash_set(merge_b->conflicted_paths, victim_abspath,
> > APR_HASH_KEY_STRING, victim_abspath);
> > }
> >
> > but I think the real fix is to identify which call to tree_conflict is
> > passing victim_path with the wrong lifetime.
>
> So I have to fix two callers to get the test to pass under valgrind:
> merge_file_deleted:2078 and merge_dir_deleted:2424. There are lots of
> other callers of tree_conflict that don't appear to duplicate the path,
> including some in merge_file_deleted and merge_dir_deleted, and I don't
> need to change them to avoid valgrind error. This is partly because the
> regression tests don't cover all the code paths, but some of the other
> callers are invoked and don't have to change.
>
> So I don't know what to do. Is tree_conflict responsible for
> duplicating the path, or is the caller responsible? There is also
> tree_conflict_on_add which is very similar.
I think your patch makes sense as-is. This code creates the conflicted_paths
table and should ensure that anything referenced from the table has the
same lifetime as the table itself.
Received on 2011-11-21 13:34:54 CET