[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1204489 - /subversion/trunk/subversion/libsvn_client/merge.c

From: Philip Martin <philip.martin_at_wandisco.com>
Date: Wed, 23 Nov 2011 13:04:55 +0000

Philip Martin <philip.martin_at_wandisco.com> writes:

> julianfoad_at_apache.org writes:
>
>> Author: julianfoad
>> Date: Mon Nov 21 13:38:13 2011
>> New Revision: 1204489
>>
>> URL: http://svn.apache.org/viewvc?rev=1204489&view=rev
>> Log:
>> Move more variables to a smaller scope and fix uninitialized output
>> parameters in callback functions. The variables previously had
>> initializers which were only necessary because the callbacks didn't
>> always initialize all their outputs as they should. This change might be
>> expected to cause a behaviour change, since the initialization was
>> previously happening only once before the first iteration of the loop.
>> I have checked fairly thoroughly and it seems in the cases where
>> outputs weren't initialized they weren't tested or the loop terminated
>> or there was some other compensating effect.
>
>> @@ -6726,9 +6735,6 @@ do_file_merge(svn_mergeinfo_catalog_t re
>> apr_pool_t *scratch_pool)
>> {
>> apr_array_header_t *remaining_ranges;
>> - svn_wc_notify_state_t prop_state = svn_wc_notify_state_unknown;
>> - svn_wc_notify_state_t text_state = svn_wc_notify_state_unknown;
>> - svn_boolean_t tree_conflicted = FALSE;
>> svn_client_ctx_t *ctx = merge_b->ctx;
>> const char *mergeinfo_path;
>> svn_merge_range_t range;
>> @@ -6855,6 +6861,8 @@ do_file_merge(svn_mergeinfo_catalog_t re
>> svn_string_t *pval;
>> const char *mimetype1, *mimetype2;
>> apr_array_header_t *propchanges;
>> + svn_wc_notify_state_t prop_state, text_state;
>> + svn_boolean_t tree_conflicted = TRUE;
>>
>> svn_pool_clear(iterpool);
>
> valgrind is giving me an unitialised memory read error in notify() for
> the property state during update_tests.py 36.
>
> do_file_merge calls merge_file_changed which calls merge_props_changed
> and inside merge_props_changed props->nelts is zero so the state is not
> set.
>
> Do we solve this by initialising prop_state? To what value? Or do we
> make merge_props_changed set it when props->nels is zero? To what value?

Also, looking at merge_file_changed: after the call to
merge_props_changed a tree conflict causes a return without setting text
state. This code has so many branches it's hard to determine that all
paths set these values. Perhaps initialising them early is better?

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com
Received on 2011-11-23 14:05:32 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.