Dan,
Yes we are not making a deep copy here. Till r26505 we had a
inconsistent code of duplicating curr->path and not doing so for
curr->propval. After careful examination I leaned towards shallow copy.
Currently shallow copy does not cause any problem and eventually won't,
unless someone keeps reference to pointers of *last* 'merge_path_t'
*across* code.
I would lean towards a 'Doc string stating the shallow copy behaviour'
rather than deep copying things.
Thanks.
With regards
Kamesh Jayachandran
Daniel Rall wrote:
> We're not making a deep copy of merge_path_t's fields here (e.g. for
> foo->path). Does that currently result in a problem (e.g. due to
> differing pool lifetime)? (It certainly could later when others
> modify the code.)
>
> On Mon, 10 Sep 2007, kameshj@tigris.org wrote:
> ...
>
>> Code Cleanup.
>>
>> * subversion/libsvn_client/merge.c
>> (insert_child_to_merge): Instead of 'member by member copy',
>> follow *ptr1=*ptr2 style of copy, so that things look simple.
>>
> ...
>
>> --- trunk/subversion/libsvn_client/merge.c (original)
>> +++ trunk/subversion/libsvn_client/merge.c Mon Sep 10 04:57:25 2007
>> @@ -3335,12 +3335,7 @@
>> merge_path_t *);
>> merge_path_t *curr_copy = apr_palloc(children_with_mergeinfo->pool,
>> sizeof(*curr_copy));
>> - curr_copy->path = curr->path;
>> - curr_copy->missing_child = curr->missing_child;
>> - curr_copy->switched = curr->switched;
>> - curr_copy->has_noninheritable = curr->has_noninheritable;
>> - curr_copy->absent = curr->absent;
>> - curr_copy->propval = curr->propval;
>> + *curr_copy = *curr;
>> APR_ARRAY_PUSH(children_with_mergeinfo, merge_path_t *) = curr_copy;
>>
>> /* Move all elements from INSERT_INDEX to the end of the array forward
>> @@ -3350,24 +3345,12 @@
>> merge_path_t *prev;
>> curr = APR_ARRAY_IDX(children_with_mergeinfo, j, merge_path_t *);
>> if (j == insert_index)
>> - {
>> - curr->path = insert_element->path;
>> - curr->missing_child = insert_element->missing_child;
>> - curr->switched = insert_element->switched;
>> - curr->has_noninheritable = insert_element->has_noninheritable;
>> - curr->absent = insert_element->absent;
>> - curr->propval = insert_element->propval;
>> - }
>> + *curr = *insert_element;
>> else
>> {
>> prev = APR_ARRAY_IDX(children_with_mergeinfo, j - 1,
>> merge_path_t *);
>> - curr->path = prev->path;
>> - curr->missing_child = prev->missing_child;
>> - curr->switched = prev->switched;
>> - curr->has_noninheritable = prev->has_noninheritable;
>> - curr->absent = prev->absent;
>> - curr->propval = prev->propval;
>> + *curr = *prev;
>> }
>> }
>> }
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Sep 11 08:50:01 2007