On Wed, 16 Mar 2005, Philip Martin wrote:
> lundblad@tigris.org writes:
>
> > Author: lundblad
> > Date: Sat Mar 12 19:14:11 2005
> > New Revision: 13393
>
> > +/* Implements svn_ra_reporter2_t->link_path. */
> > +static svn_error_t *
> > +reporter_link_path (void *report_baton, const char *path, const char *url,
> > + svn_revnum_t revision, svn_boolean_t start_empty,
> > + const char *lock_token, apr_pool_t *pool)
> > +{
> > + report_baton_t *rb = report_baton;
> > + const char *ancestor;
> > + apr_size_t len;
> > +
> > + /* Get the common ancestor of the URL and our current ancestor. */
>
> That comment doesn't add much.
>
Heh, good point. I hate those commets when I read them:-) Will drop.
> > + ancestor = svn_path_get_longest_ancestor (url, rb->ancestor, pool);
> > +
> > + /* If we got a shorter ancestor, truncate our current ancestor. */
>
> That comment tells me what is happening, but not why.
>
OK, I'll add some explanation of the "algorithm" somewhere.
> > + len = strlen (ancestor);
> > + if (len < strlen (rb->ancestor))
> > + rb->ancestor[len] = '\0';
>
> I find that a bit confusing, I suppose the main aim is to to ensure
> that rb->ancestor is allocated from a pool with a correct lifetime,
> but I don't understand why you don't simply pass rb->pool into
> svn_path_get_longest_ancestor. Is rb->pool the wrong pool? Is the
> memory saved by reusing rb->ancestor important?
>
svn_path_get_longest_ancestor will always copy the common ancestor, even
if it is identical to one of the paths. (It has to, since it returns
non-const char*) This means a copy of the URL for each call, which seems
unneccasry to me when we can just truncate the path. I'll ad a comment
explaining that.
thanx,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 16 09:18:01 2005