On Sat, 22 Jan 2005 kfogel@collab.net wrote:
> lundblad@tigris.org writes:
> > Author: lundblad
> > Date: Thu Jan 20 15:05:59 2005
> > New Revision: 12801
> >
> > Log:
> > New RA API.
>
> So Peter, I'm not going to send my review all at once -- instead I'll
> ask questions in multiple mails. (I'm not done with the review yet
> anyway, but even if I were, multiple mails might be easier.)
>
Yes, thanks.
> > --- trunk/subversion/include/svn_ra.h (original)
> > +++ trunk/subversion/include/svn_ra.h Thu Jan 20 15:05:59 2005
> > @@ -277,16 +277,551 @@
> >
> > /*----------------------------------------------------------------------*/
> >
> > -/** The RA Library.
> > +/* Public Interfaces. */
> > +
> > +/**
> > + * @since New in 1.2.
> > + *
> > + * A repository access session. This object is used to perform requests
> > + * to a repository, identified by an URL.
> > + */
> > +typedef struct svn_ra_session_t svn_ra_session_t;
> > +
> > +/**
> > + * @since New in 1.2.
> > + *
> > + * Open a repository session to @a repos_url. Return an opaque object
> > + * representing this session in @a *session_p, allocated in @a pool.
> > + *
> > + * @a callbacks/@a callback_baton is a table of callbacks provided by the
> > + * client; see @c svn_ra_callbacks_t above.
> > + *
> > + * @a config is a hash mapping <tt>const char *</tt> keys to
> > + * @c svn_config_t * values. For example, the @c svn_config_t for the
> > + * "~/.subversion/config" file is under the key "config".
> > + *
> > + * All RA requests require a session; they will continue to
> > + * use @a pool for memory allocation.
> > + */
> > +svn_error_t *svn_ra_open (svn_ra_session_t **session_p,
> > + const char *repos_URL,
> > + const svn_ra_callbacks_t *callbacks,
> > + void *callback_baton,
> > + apr_hash_t *config,
> > + apr_pool_t *pool);
>
> This doc string says that RA requests will "continue to use @a pool
> for memory allocation", referring the pool parameter above, which was
> passed to svn_ra_open() and presumably is preserved in the session
> object.
>
> But this seems to contradict the doc strings of the svn_ra_* functions
> themselves. For examples:
>
...
> If that initial pool is used for much beyond the allocation of the
> session object, then won't we have a memory leak problem when ra
> functions are called repeatedly in loops? Because even if the caller
> dutifully passes a reuseable pool to each svn_ra_foo() call, some
> allocation would still be happening in that original pool, beyond the
> control of the calling loop.
>
I think that sentence stems from an early time, but I don't know. Did the
RA functions have separate pools back - when?
The current situation is like this:
The pool passed to open is a session pool. Things allocated there will
live during the session. The pools passed to the other functions are used
for the work of those functions. But those functions sometimes need to
allocate things in the session pool. An example is the authenticated
username. Well, your point is that the docstring is unclear and I aggree.
I am accumulating such changes for some days. I am pretty sure I will get
some trivial things from your review. This big wrapper and deprecation
made my head spin somewhat and I probably missed some reindentation and
such.
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jan 22 18:31:49 2005