[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1148566 - /subversion/trunk/subversion/libsvn_client/merge.c

From: Greg Stein <gstein_at_gmail.com>
Date: Tue, 19 Jul 2011 20:48:02 -0400

On Tue, Jul 19, 2011 at 20:29, Greg Stein <gstein_at_gmail.com> wrote:
> On Tue, Jul 19, 2011 at 20:10, Greg Stein <gstein_at_gmail.com> wrote:
>> On Tue, Jul 19, 2011 at 17:51,  <stsp_at_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
Received on 2011-07-20 02:48:38 CEST

This is an archived mail posted to the Subversion Dev mailing list.