Re: svn commit: r13393 - in branches/locking/subversion: include libsvn_client libsvn_wc

Peter N. Lundblad wrote:
> On Wed, 16 Mar 2005, Philip Martin wrote:
>>lundblad@tigris.org writes:
>>>+ ancestor = svn_path_get_longest_ancestor (url, rb->ancestor, pool);
[...]
>>>+ 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,

"svn_path_get_longest_ancestor" already makes a copy of the URL for each call;
you cannot avoid that.

> which seems unneccasry to me when we can just truncate the path.

Do you mean it would otherwise require another deep copy of the new string to
move it from "pool" into the report baton's pool? Did you follow what Philip
was saying? It looks like those four lines of code quoted above could be
replaced by the single line:

     rb->ancestor = svn_path_get_longest_ancestor (url, rb->ancestor, rb->pool);

This causes the new string to be allocated in the correct pool in the first place.

However, this would mean that any temporary allocations performed within
"svn_path_get_longest_ancestor" would accumulate in "rb->pool"; perhaps that is
what you are purposefully avoiding.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 16 14:00:39 2005