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

RE: svn commit: r36613 - trunk/subversion/libsvn_client

From: Bert Huijben <rhuijben_at_sharpsvn.net>
Date: Tue, 17 Mar 2009 09:45:13 +0100

> -----Original Message-----
> From: Paul T. Burba [mailto:pburba_at_collab.net]
> Sent: Tuesday, March 17, 2009 1:23 AM
> To: svn_at_subversion.tigris.org
> Subject: svn commit: r36613 - trunk/subversion/libsvn_client
>
> Author: pburba
> Date: Mon Mar 16 17:22:43 2009
> New Revision: 36613
>
> Log:
> Avoid potential segfaults caused by NULL elements in the
> CHILDREN_WITH_MERGEINFO array by removing elements rather than setting
> them
> to NULL.

        Hi,

It looks like this test generates the following failure on neon (and
probably serf):
FAIL: merge_authz_tests.py 1: skipped paths get overriding mergeinfo

(From
http://crest.ics.uci.edu/buildbot/builders/x86-macosx-gnu%20shared/builds/17
8/steps/Test%20fsfs%2Bra_neon/logs/testlog/text):

CMD: svn proplist --verbose
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/C
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/ome
ga svn-test-
work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E/bet
a svn-test-work/working_copies/merge_authz_test
s-1.restricted/A_COPY_2/mu
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/gamma
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E/alp
ha
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/ch
i
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/lambd
a svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E
svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/F
svn-test-work/working_copies/mer
ge_authz_tests-1.restricted/A_COPY_2 --config-dir
/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/svn
-test-work/local_tmp/config --password rayjandom --no-auth-cache --username
jrandom <TIME = 0.086657>
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/om
ega':
  svn:mergeinfo
    /A/D/H/omega:5-8
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D':
  svn:mergeinfo
    /A/D:5-8*
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H':
  svn:mergeinfo
    /A/D/H:5-8*
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/D/H/ch
i':
  svn:mergeinfo
    /A/D/H/chi:5-8
Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2/B/E':
  svn:mergeinfo

Properties on
'svn-test-work/working_copies/merge_authz_tests-1.restricted/A_COPY_2':
  svn:mergeinfo
    /A:5-8
=============================================================
Expected 'gamma' and actual 'gamma' in disk tree are different!
=============================================================
EXPECTED NODE TO BE:
=============================================================
 * Node name: gamma
    Path: __SVN_ROOT_NODE/D/gamma
    Contents: This is the file 'gamma'.

    Properties: {'svn:mergeinfo': '/A/D/gamma:5-8'}
    Attributes: {}
    Children: None (node is probably a file)
=============================================================
ACTUAL NODE FOUND:
=============================================================
 * Node name: gamma
    Path: __SVN_ROOT_NODE/D/gamma
    Contents: This is the file 'gamma'.

    Properties: {}
    Attributes: {}
    Children: None (node is probably a file)
Unequal at node gamma
Unequal at node D
ACTUAL DISK TREE:
svntest.wc.State('', {
  'C' : Item(),
  'B' : Item(),
  'B/E' : Item(props={'svn:mergeinfo':''}),
  'B/E/beta' : Item(contents="This is the file 'beta'.\n"),
  'B/E/alpha' : Item(contents="This is the file 'alpha'.\n"),
  'B/lambda' : Item(contents="This is the file 'lambda'.\n"),
  'B/F' : Item(),
  'D' : Item(props={'svn:mergeinfo':'/A/D:5-8*'}),
  'D/H' : Item(props={'svn:mergeinfo':'/A/D/H:5-8*'}),
  'D/H/omega' : Item(contents="New content",
props={'svn:mergeinfo':'/A/D/H/omega:5-8'}),
  'D/H/chi' : Item(contents="This is the file 'chi'.\n",
props={'svn:mergeinfo':'/A/D/H/chi:5-8'}),
  'D/gamma' : Item(contents="This is the file 'gamma'.\n"),
  'mu' : Item(contents="This is the file 'mu'.\n"),
  '.' : Item(props={'svn:mergeinfo':'/A:5-8'}),
})
EXCEPTION: SVNTreeUnequal
Traceback (most recent call last):
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/main.py", line 1120, in run
    rc = self.pred.run(sandbox)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/testcase.py", line 185, in run
    return self._delegate.run(sandbox)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/testcase.py", line 185, in run
    return self._delegate.run(sandbox)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/testcase.py", line 114, in run
    return self.func(sandbox)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/me
rge_authz_tests.py", line 260, in mergeinfo_and_skipped_paths
    None, 1, 0)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/actions.py", line 788, in run_and_verify_merge
    b_baton, check_props, dry_run)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/actions.py", line 916, in run_and_verify_merge2
    check_props)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/actions.py", line 615, in verify_update
    singleton_handler_b, b_baton)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/tree.py", line 626, in compare_trees
    singleton_handler_b, b_baton)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/tree.py", line 626, in compare_trees
    singleton_handler_b, b_baton)
  File
"/Users/lgo/slavedir/osx10.4-gcc4.0.1-ia32/build/subversion/tests/cmdline/sv
ntest/tree.py", line 601, in compare_trees
    raise SVNTreeUnequal
SVNTreeUnequal
FAIL: merge_authz_tests.py 1: skipped paths get overriding mergeinfo
>
> Found by: Mark Eichin <eichin_at_gmail.com>
>
> See http://svn.haxx.se/dev/archive-2009-03/0382.shtml.
>
> * subversion/libsvn_client/merge.c
> (CHILDREN_WITH_MERGEINFO ARRAY): Tweak this global comment.
> (populate_remaining_ranges, drive_merge_report_editor,
> do_directory_merge):
> Check for NULL children_with_mergeinfo elements with SVN_ERR_ASSERT.
> (remove_child_with_mergeinfo): New.
> (remove_absent_children, remove_children_with_deleted_mergeinfo): Use
> remove_child_with_mergeinfo() to truly remove elements rather than
> setting
> them to NULL.
>
> Modified:
> trunk/subversion/libsvn_client/merge.c
>
> Modified: trunk/subversion/libsvn_client/merge.c
> URL:
> http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/merge.c
> ?pathrev=36613&r1=36612&r2=36613
> =======================================================================
> =======
> --- trunk/subversion/libsvn_client/merge.c Mon Mar 16 16:57:46 2009
> (r36612)
> +++ trunk/subversion/libsvn_client/merge.c Mon Mar 16 17:22:43 2009
> (r36613)
> @@ -131,7 +131,8 @@
> *
> * CHILDREN_WITH_MERGEINFO is initially created by
> get_mergeinfo_paths()
> * and outside of that function and its helpers should always meet the
> - * criteria dictated in get_mergeinfo_paths()'s doc string.
> + * criteria dictated in get_mergeinfo_paths()'s doc string. The
> elements
> + * of CHILDREN_WITH_MERGINFO should never be NULL.
> */
>
> /*--------------------------------------------------------------------
> ---*/
> @@ -3269,7 +3270,8 @@ populate_remaining_ranges(apr_array_head
> svn_client__merge_path_t *parent = NULL;
>
> /* If the path is absent don't do subtree merge either. */
> - if (!child || child->absent)
> + SVN_ERR_ASSERT(child);
> + if (child->absent)
> continue;
>
> svn_pool_clear(iterpool);
> @@ -3634,6 +3636,37 @@ make_merge_conflict_error(const char *ta
> r->start, r->end, svn_path_local_style(target_wcpath, pool));
> }
>
> +/* Remove the element at INDEX from the array CHILDREN_WITH_MERGEINFO.
> + If INDEX is not a valid element of CHILDREN_WITH_MERGEINFO do
> nothing. */
> +static void
> +remove_child_with_mergeinfo(apr_array_header_t
> *children_with_mergeinfo,
> + int index)
> +{
> + /* Do we have a valid index? */
> + if (index >= 0 && index < children_with_mergeinfo->nelts)
> + {
> + if (index == (children_with_mergeinfo->nelts - 1))
> + {
> + /* Deleting the last or only element in an array is easy. */
> + apr_array_pop(children_with_mergeinfo);
> + }
> + else
> + {
> + int remainder = children_with_mergeinfo->nelts - 1 - index;
> + svn_client__merge_path_t *deleted_child =
> + APR_ARRAY_IDX(children_with_mergeinfo, index,
> + svn_client__merge_path_t *);
> + svn_client__merge_path_t *next_child =
> + APR_ARRAY_IDX(children_with_mergeinfo, index + 1,
> + svn_client__merge_path_t *);
> +
> + memmove(deleted_child, next_child,
> + children_with_mergeinfo->elt_size * remainder);
> + --(children_with_mergeinfo->nelts);
> + }
> + }
> +}
> +
> /* Helper for do_directory_merge().
>
> TARGET_WCPATH is a directory and CHILDREN_WITH_MERGEINFO is filled
> @@ -3657,15 +3690,13 @@ remove_absent_children(const char *targe
> svn_client__merge_path_t *child =
> APR_ARRAY_IDX(children_with_mergeinfo,
> i, svn_client__merge_path_t *);
> - if (child
> - && (child->absent || child->scheduled_for_deletion)
> + if ((child->absent || child->scheduled_for_deletion)
> && svn_path_is_ancestor(target_wcpath, child->path))
> {
> if (notify_b->skipped_paths)
> apr_hash_set(notify_b->skipped_paths, child->path,
> APR_HASH_KEY_STRING, NULL);
> - APR_ARRAY_IDX(children_with_mergeinfo, i,
> - svn_client__merge_path_t *) = NULL;
> + remove_child_with_mergeinfo(children_with_mergeinfo, i);
> }
> }
> }
> @@ -3695,13 +3726,12 @@ remove_children_with_deleted_mergeinfo(m
> svn_client__merge_path_t *child =
> APR_ARRAY_IDX(notify_b->children_with_mergeinfo,
> i, svn_client__merge_path_t *);
> - if (child
> - && apr_hash_get(merge_b->paths_with_deleted_mergeinfo,
> - child->path,
> - APR_HASH_KEY_STRING))
> + if (apr_hash_get(merge_b->paths_with_deleted_mergeinfo,
> + child->path,
> + APR_HASH_KEY_STRING))
> {
> - APR_ARRAY_IDX(notify_b->children_with_mergeinfo, i,
> - svn_client__merge_path_t *) = NULL;
> + remove_child_with_mergeinfo(notify_b-
> >children_with_mergeinfo,
> + i);
> }
> }
> }
> @@ -3822,7 +3852,7 @@ drive_merge_report_editor(const char *ta
> see 'THE CHILDREN_WITH_MERGEINFO ARRAY'. */
> child = APR_ARRAY_IDX(children_with_mergeinfo, 0,
> svn_client__merge_path_t *);
> -
> + SVN_ERR_ASSERT(child);
> if (child->remaining_ranges->nelts == 0)
> {
> /* The merge target doesn't need anything merged. */
> @@ -3899,7 +3929,8 @@ drive_merge_report_editor(const char *ta
> int parent_index;
> svn_boolean_t nearest_parent_is_target;
>
> - if (!child || child->absent)
> + SVN_ERR_ASSERT(child);
> + if (child->absent)
> continue;
>
> /* Find this child's nearest wc ancestor with mergeinfo. */
> @@ -6330,7 +6361,8 @@ do_directory_merge(const char *url1,
> svn_client__merge_path_t *child =
> APR_ARRAY_IDX(notify_b-
> >children_with_mergeinfo, i,
> svn_client__merge_path_t *);
> - if (!child || child->absent)
> + SVN_ERR_ASSERT(child);
> + if (child->absent)
> continue;
>
> if (strlen(child->path) == merge_target_len)
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageI
> d=1336228

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1339304
Received on 2009-03-17 09:45:41 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.