You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2007/04/25 21:12:14 UTC
Incorrect doc string for svn_log_message_receiver_t's pool parameter?
svn_log_message_receiver_t documents its POOL parameter as:
* Use @a pool for all allocation. (If the caller is iterating over log
* messages, invoking this receiver on each, we recommend the standard
* pool loop recipe: create a subpool, pass it as @a pool to each call,
* clear it after each iteration, destroy it after the loop is done.)
However, in svn_repos_get_logs3() (called indirectly over ra_local),
we invoke the function pointer parameter RECEIVER with an iterpool (a
sub-pool used for temporary allocation in loops). This means that any
data allocated in the POOL passed to this log receiver is going to be
cleared, and that a pool must be stashed in the receiver's BATON.
This means that either the documentation for
svn_log_message_receiver_t is incorrect, or that
svn_repos_get_logs3()'s usage of its receiver is wrong.
Which is it?
Re: Incorrect doc string for svn_log_message_receiver_t's pool parameter?
Posted by Karl Fogel <kf...@red-bean.com>.
Daniel Rall <dl...@collab.net> writes:
> svn_log_message_receiver_t's doc string says "Use @a pool for all
> allocation." But this isn't appropriate if you want to allocate some
> memory which has a lifetime past one (looped) invocation of the
> receiver -- the allocation is clearly only temporary. So, should the
> doc string instead read "Use @a pool for temporary allocations"?
Aaah, I see what you mean, yes. Fixed in r24786.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Incorrect doc string for svn_log_message_receiver_t's pool parameter?
Posted by Daniel Rall <dl...@collab.net>.
On Wed, 25 Apr 2007, Karl Fogel wrote:
> Daniel Rall <dl...@collab.net> writes:
> > svn_log_message_receiver_t documents its POOL parameter as:
> >
> > * Use @a pool for all allocation. (If the caller is iterating over log
> > * messages, invoking this receiver on each, we recommend the standard
> > * pool loop recipe: create a subpool, pass it as @a pool to each call,
> > * clear it after each iteration, destroy it after the loop is done.)
> >
> > However, in svn_repos_get_logs3() (called indirectly over ra_local),
> > we invoke the function pointer parameter RECEIVER with an iterpool (a
> > sub-pool used for temporary allocation in loops). This means that any
> > data allocated in the POOL passed to this log receiver is going to be
> > cleared, and that a pool must be stashed in the receiver's BATON.
> >
> > This means that either the documentation for
> > svn_log_message_receiver_t is incorrect, or that
> > svn_repos_get_logs3()'s usage of its receiver is wrong.
> >
> > Which is it?
>
> I'm not sure I understand. It seems to me that the invocation of
> RECEIVER in svn_repos_get_logs3(), via send_change_rev(), is following
> the policy recommended in the svn_log_message_receiver_t doc string:
> Pass an iterpool to the receiver, clear it between invocations of the
> receiver, destroy it when done. Is this not what happens in this
> passage from svn_repos_get_logs3()?:
>
> for (i = 0; i < send_count; ++i)
> {
> svn_revnum_t rev = hist_start + i;
>
> svn_pool_clear(subpool);
> if (start > end)
> rev = hist_end - i;
> SVN_ERR(send_change_rev(rev, fs,
> discover_changed_paths,
> authz_read_func, authz_read_baton,
> receiver, receiver_baton, subpool));
> }
> svn_pool_destroy(subpool);
...
svn_log_message_receiver_t's doc string says "Use @a pool for all
allocation." But this isn't appropriate if you want to allocate some
memory which has a lifetime past one (looped) invocation of the
receiver -- the allocation is clearly only temporary. So, should the
doc string instead read "Use @a pool for temporary allocations"?
Re: Incorrect doc string for svn_log_message_receiver_t's pool parameter?
Posted by Karl Fogel <kf...@red-bean.com>.
Daniel Rall <dl...@collab.net> writes:
> svn_log_message_receiver_t documents its POOL parameter as:
>
> * Use @a pool for all allocation. (If the caller is iterating over log
> * messages, invoking this receiver on each, we recommend the standard
> * pool loop recipe: create a subpool, pass it as @a pool to each call,
> * clear it after each iteration, destroy it after the loop is done.)
>
> However, in svn_repos_get_logs3() (called indirectly over ra_local),
> we invoke the function pointer parameter RECEIVER with an iterpool (a
> sub-pool used for temporary allocation in loops). This means that any
> data allocated in the POOL passed to this log receiver is going to be
> cleared, and that a pool must be stashed in the receiver's BATON.
>
> This means that either the documentation for
> svn_log_message_receiver_t is incorrect, or that
> svn_repos_get_logs3()'s usage of its receiver is wrong.
>
> Which is it?
I'm not sure I understand. It seems to me that the invocation of
RECEIVER in svn_repos_get_logs3(), via send_change_rev(), is following
the policy recommended in the svn_log_message_receiver_t doc string:
Pass an iterpool to the receiver, clear it between invocations of the
receiver, destroy it when done. Is this not what happens in this
passage from svn_repos_get_logs3()?:
for (i = 0; i < send_count; ++i)
{
svn_revnum_t rev = hist_start + i;
svn_pool_clear(subpool);
if (start > end)
rev = hist_end - i;
SVN_ERR(send_change_rev(rev, fs,
discover_changed_paths,
authz_read_func, authz_read_baton,
receiver, receiver_baton, subpool));
}
svn_pool_destroy(subpool);
return SVN_NO_ERROR;
or in the alternative code later in the function:
/* Loop through all the revisions in the range and add any
where a path was changed to the array, or if they wanted
history in reverse order just send it to them right away.
*/
for (current = hist_end;
current >= hist_start && any_histories_left;
current = next_history_rev(histories))
{
[...]
svn_pool_clear(subpool);
[...]
SVN_ERR(send_change_rev([...], subpool));
[...]
}
svn_pool_destroy(subpool);
return SVN_NO_ERROR;
?
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org