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

Re: Incorrect doc string for svn_log_message_receiver_t's pool parameter?

From: Karl Fogel <kfogel_at_red-bean.com>
Date: 2007-04-25 23:27:11 CEST

Daniel Rall <dlr@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
Received on Wed Apr 25 23:27:22 2007

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.