On Mon, Nov 21, 2011 at 12:12:56PM +0000, Philip Martin wrote: > Philip Martin <philip.mar...@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.