subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bert Huijben" <b...@qqmail.nl>
Subject RE: svn commit: r1330444 - in /subversion/trunk/subversion: libsvn_client/merge.c tests/cmdline/merge_tests.py
Date Wed, 25 Apr 2012 19:43:37 GMT


> -----Original Message-----
> From: pburba@apache.org [mailto:pburba@apache.org]
> Sent: woensdag 25 april 2012 19:58
> To: commits@subversion.apache.org
> Subject: svn commit: r1330444 - in /subversion/trunk/subversion:
> libsvn_client/merge.c tests/cmdline/merge_tests.py
> 
> Author: pburba
> Date: Wed Apr 25 17:57:30 2012
> New Revision: 1330444
> 
> URL: http://svn.apache.org/viewvc?rev=1330444&view=rev
> Log:
> Fix issue #4169 'added subtrees with non-inheritable mergeinfo cause
> spurious subtree mergeinfo'.
> 
> This fixes a bug julianf noted in r1205867.
> 
> * subversion/libsvn_client/merge.c
> 
>   (notification_receiver_baton_t): Clarify what is and isn't stored in the
>    added_abspaths member.
> 
>   (notification_receiver): Only stash the roots of added subtrees rather
>    than tracking every added subtree excepting the immediate children of
>    added roots (which is what happened previously).  There is no need to
>    track those other added subtrees (and as the test below demonstrates,
>    there are edge cases where spurious subtree mergeinfo can be created).
> 
> * subversion/tests/cmdline/merge_tests.py
> 
>   (merge_with_added_subtrees_with_mergeinfo): New.
> 
>   (test_list): Add merge_with_added_subtrees_with_mergeinfo.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_client/merge.c
>     subversion/trunk/subversion/tests/cmdline/merge_tests.py
> 
> Modified: subversion/trunk/subversion/libsvn_client/merge.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/merge
> .c?rev=1330444&r1=1330443&r2=1330444&view=diff
> =================================================================
> =============
> --- subversion/trunk/subversion/libsvn_client/merge.c (original)
> +++ subversion/trunk/subversion/libsvn_client/merge.c Wed Apr 25 17:57:30
> 2012
> @@ -2941,30 +2941,47 @@ notification_receiver(void *baton, const
> 
>        if (notify->action == svn_wc_notify_update_add)
>          {
> -          svn_boolean_t is_root_of_added_subtree = FALSE;
> -          const char *added_path = apr_pstrdup(notify_b->pool,
> -                                               notify_abspath);
> -          const char *added_path_parent = NULL;
> +          svn_boolean_t root_of_added_subtree = TRUE;
> 
>            /* Stash the root path of any added subtrees. */
>            if (notify_b->added_abspaths == NULL)
>              {
> +              /* The first added path is always a root. */
>                notify_b->added_abspaths = apr_hash_make(notify_b->pool);
> -              is_root_of_added_subtree = TRUE;
>              }
>            else
>              {
> -              added_path_parent = svn_dirent_dirname(added_path, pool);
> -              /* ### Bug. Testing whether its immediate parent is in the
> -               * hash isn't enough: this is letting every other level of
> -               * the added subtree hierarchy into the hash. */
> -              if (!apr_hash_get(notify_b->added_abspaths, added_path_parent,
> -                                APR_HASH_KEY_STRING))
> -                is_root_of_added_subtree = TRUE;
> -            }
> -          if (is_root_of_added_subtree)
> -            apr_hash_set(notify_b->added_abspaths, added_path,
> -                         APR_HASH_KEY_STRING, added_path);
> +              const char *added_path_parent =
> +                svn_dirent_dirname(notify_abspath, pool);
> +              apr_pool_t *iterpool = svn_pool_create(pool);
> +
> +              /* Is NOTIFY->PATH the root of an added subtree? */
> +              while (strcmp(notify_b->merge_b->target->abspath,
> +                            added_path_parent))
> +                {
> +                  if (apr_hash_get(notify_b->added_abspaths,
> +                                   added_path_parent,
> +                                   APR_HASH_KEY_STRING))
> +                    {
> +                      root_of_added_subtree = FALSE;
> +                      break;
> +                    }
> +
> +                  svn_pool_clear(iterpool);
> +                  added_path_parent = svn_dirent_dirname(
> +                    added_path_parent, iterpool);

This will use unallocated memory when iterating.

The first loop is safe, but after the argument to svn_dirent_dirname will be pointing to memory
in the just cleared pool.

Assuming the path is of a normal length the fix is to just not clear the pool.
> +                }
> +
> +              svn_pool_destroy(iterpool);

Because the pool is destroyed here anyway.
> +            }
> +
> +          if (root_of_added_subtree)
> +            {
> +              const char *added_root_path = apr_pstrdup(notify_b->pool,
> +                                                        notify_abspath);
> +              apr_hash_set(notify_b->added_abspaths, added_root_path,
> +                           APR_HASH_KEY_STRING, added_root_path);
> +            }
>          }
> 
>        if (notify->action == svn_wc_notify_update_delete
> 

	Bert


Mime
View raw message