On Tue, Jul 19, 2011 at 20:29, Greg Stein <gst...@gmail.com> wrote: > On Tue, Jul 19, 2011 at 20:10, Greg Stein <gst...@gmail.com> wrote: >> On Tue, Jul 19, 2011 at 17:51, <s...@apache.org> wrote: >>>... >>> * subversion/libsvn_client/merge.c >>> (log_noop_revs): The baton's pool is used in an iterpool pattern so it >>> must be cleared on each invocation of this function. >>> (remove_noop_subtree_ranges): Make the baton's pool a proper subpool >>> to avoid clearing unrelated data in log_noop_revs(). >> >> Why does the baton have a pool at all? The pool passed to >> log_noop_revs() *is* already a scratch_pool. If that pool is not >> getting cleared on each iteration, then we need to fix the caller. >> >> The log_baton shouldn't even have a pool, I think. >> >> I believe the above change is the wrong fix for this issue. The >> caller(s) should be corrected (and remove that spurious pool). > > Ugh. There isn't a single svn_pool_clear() in libsvn_ra_serf/log.c. > > That should be fixed. log_context_t should grow an "item_pool" that is > cleared on each call to push_state(ITEM). Then all the log_entry stuff > is placed into that pool, and passed to the receiver. That item_pool > would also be the scratch_pool passed to the receiver. > > The log_info_t would lose its pool member. (and it really should be > called something like log_element_t since it always corresponds > directly to an XML element) > > Thoughts?
Bert solved the underlying problem in r1148588. However, I think that we can iterate on that change to clarify pool usage, and clear a pool (rather than create/destroy). e.g my suggestions above. Note that Bert's change fixes all log usage: rather than just 'svn merge', it also handles 'svn log' (dunno where else we fetch logs). We figured to wait for your thoughts before another iteration. Cheers, -g
