You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@gmail.com> on 2009/10/31 11:04:57 UTC

Re: svn commit: r40306 - trunk/subversion/libsvn_wc

On Thu, Oct 29, 2009 at 19:59, Paul T. Burba <pb...@collab.net> wrote:
>...
> +++ trunk/subversion/libsvn_wc/copy.c   Thu Oct 29 16:59:24 2009        (r40306)
>...
> @@ -355,13 +294,36 @@ determine_copyfrom_info(const char **cop
>                                         src_copyfrom_relpath, result_pool);
>       rev = src_copyfrom_rev;
>     }
> -  else
> +  else if (status == svn_wc__db_status_added
> +           || status == svn_wc__db_status_obstructed_add)
>     {
>       /* ...But if this file is merely the descendant of an explicitly
>          copied/moved directory, we need to do a bit more work to
>          determine copyfrom_url and copyfrom_rev. */
> -      SVN_ERR(get_copyfrom_url_rev_via_parent(&url, &rev, db, src_abspath,
> -                                              scratch_pool, scratch_pool));
> +      const char *op_root_abspath;
> +      const char *original_repos_relpath;
> +      const char *original_root_url;
> +      svn_revnum_t original_revision;
> +      const char *op_root_rel_path;
> +
> +      SVN_ERR(svn_wc__db_scan_addition(&status, &op_root_abspath,
> +                                       NULL, NULL, NULL,
> +                                       &original_repos_relpath,
> +                                       &original_root_url, NULL,
> +                                       &original_revision,

Why not directly query into &rev ?

> +                                       db, src_abspath,
> +                                       scratch_pool, scratch_pool));
> +      url = svn_uri_join(original_root_url, original_repos_relpath,
> +                         result_pool);
> +      op_root_rel_path = svn_dirent_is_child(op_root_abspath, src_abspath,
> +                                             scratch_pool);
> +      url = svn_uri_join(url, op_root_rel_path, scratch_pool);
> +      rev = original_revision;
> +    }

url is probably going into the wrong pool here. The first join should
definitely use scratch_pool, and I'm guessing the second was probably
intended for result_pool.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2413208

Re: svn commit: r40306 - trunk/subversion/libsvn_wc

Posted by Stefan Sperling <st...@elego.de>.
On Sat, Oct 31, 2009 at 07:04:57AM -0400, Greg Stein wrote:
> On Thu, Oct 29, 2009 at 19:59, Paul T. Burba <pb...@collab.net> wrote:
> >...
> > +++ trunk/subversion/libsvn_wc/copy.c   Thu Oct 29 16:59:24 2009        (r40306)
> >...
> > @@ -355,13 +294,36 @@ determine_copyfrom_info(const char **cop
> >                                         src_copyfrom_relpath, result_pool);
> >       rev = src_copyfrom_rev;
> >     }
> > -  else
> > +  else if (status == svn_wc__db_status_added
> > +           || status == svn_wc__db_status_obstructed_add)
> >     {
> >       /* ...But if this file is merely the descendant of an explicitly
> >          copied/moved directory, we need to do a bit more work to
> >          determine copyfrom_url and copyfrom_rev. */
> > -      SVN_ERR(get_copyfrom_url_rev_via_parent(&url, &rev, db, src_abspath,
> > -                                              scratch_pool, scratch_pool));
> > +      const char *op_root_abspath;
> > +      const char *original_repos_relpath;
> > +      const char *original_root_url;
> > +      svn_revnum_t original_revision;
> > +      const char *op_root_rel_path;
> > +
> > +      SVN_ERR(svn_wc__db_scan_addition(&status, &op_root_abspath,
> > +                                       NULL, NULL, NULL,
> > +                                       &original_repos_relpath,
> > +                                       &original_root_url, NULL,
> > +                                       &original_revision,
> 
> Why not directly query into &rev ?
> 
> > +                                       db, src_abspath,
> > +                                       scratch_pool, scratch_pool));
> > +      url = svn_uri_join(original_root_url, original_repos_relpath,
> > +                         result_pool);
> > +      op_root_rel_path = svn_dirent_is_child(op_root_abspath, src_abspath,
> > +                                             scratch_pool);
> > +      url = svn_uri_join(url, op_root_rel_path, scratch_pool);
> > +      rev = original_revision;
> > +    }
> 
> url is probably going into the wrong pool here. The first join should
> definitely use scratch_pool, and I'm guessing the second was probably
> intended for result_pool.

No, because the joined result is later dupped into the result pool.
Your concerns should be addressed by r4032{1,2,3}.

Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2413225