Re: svn commit: r22765 - trunk/subversion/libsvn_client

Kamesh Jayachandran wrote:
> Daniel Rall wrote:
>> On Wed, 20 Dec 2006, Kamesh Jayachandran wrote:
>>
>>
>>> dlr@tigris.org wrote:
>>>
>>>> Modified:
>>>> trunk/subversion/libsvn_client/copy.c
>>>>
>>>> Modified: trunk/subversion/libsvn_client/copy.c
>>>> URL:
>>>> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/copy.c?pathrev=22765&r1=22764&r2=22765
>>>>
>>>> ==============================================================================
>>>>
>>>> --- trunk/subversion/libsvn_client/copy.c (original)
>>>> +++ trunk/subversion/libsvn_client/copy.c Wed Dec 20 06:06:12 2006
>>>>
>>>> /* Check that all of our SRCs exist, and all the DSTs don't. */
>>>> for (i = 0; i < copy_pairs->nelts; i++)
>>>> @@ -221,7 +220,7 @@
>>>> ((svn_client__copy_pair_t **) (copy_pairs->elts))[i];
>>>> svn_node_kind_t dst_kind, dst_parent_kind;
>>>>
>>>> - svn_pool_clear(subpool);
>>>> + svn_pool_clear(iterpool);
>>>>
>>>> /* Verify that SRC_PATH exists. */
>>>> SVN_ERR(svn_io_check_path(pair->src, &pair->src_kind, pool));
>>>>
>>> Why not this svn_io_check_path from iterpool?
>>>
>>
>> As I mentioned in a separate email, this operation allocates memory
>> for our input parameters. I want to avoid altering their values, then
>> destroying the memory holding their new values. While I don't believe
>> that those values are used now, this is a safe practice which leaves
>> the code without booby-traps (and thus easier to modify).
>>
>> - Dan
>>
> Given the call it can only modify '&pair->src_kind' as 'pair->src_kind'
> is an enum it does not need any allocation, so no need to worry abt pool
> destruction.

I think Kamesh is right on this one. The use of the pool parameter
isn't documented in svn_io.h, and looking at the internals, pool is only
used for allocations inside svn_io_check_path, not for anything that
gets returned. pair->src_kind isn't allocated, just set to an enum value.

Using iterpool here is safe.

-Hyrum