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

Re: Reviewing r34562

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Wed, 17 Dec 2008 11:15:35 +0000

Kamesh Jayachandran wrote:
> Justin Erenkrantz wrote:
> C. Michael Pilato <cmpilato_at_collab.net> wrote:
> >> r34562 (which is proposed for backport to 1.5.x). It adds code that looks
> >> like so:
> >>
> >> /* abs_added_path had better be a child of abs_target_path
> >> or something is *really* wrong. */
> >> SVN_ERR_ASSERT(svn_path_is_child(abs_target_path,
> >> abs_added_path,
> >> iterpool));
> >> common_ancestor_path =
> >> svn_path_get_longest_ancestor(abs_added_path,
> >> abs_target_path,
> >> iterpool);
> >>
> >> I'm a little confused by these two statements, though. If ABS_ADDED_PATH is
> >> a child of ABS_TARGET_PATH, then isn't the longest ancestor common to the
> >> two of them necessarily ABS_TARGET_PATH? Could this be written as:
> >>
> >> common_ancestor_path =
> >> svn_path_get_longest_ancestor(abs_added_path,
> >> abs_target_path,
> >> iterpool);
> >> SVN_ERR_ASSERT(*common_ancestor_path);
> >>
> >> ?
> >
> > Yah, this snippet struck me as being redundnant too, but I don't
> > *think* it should harm anything.
> >
> > +1 if you want to change this and merge it back for 1.5.x. -- justin
>
> Fixed in r34756
>
> With regards
> Kamesh Jayachandran

Huh? You are now asserting only that there is some common ancestor.
Previously you were asserting that ABS_ADDED_PATH was the common
ancestor. That's very different.

Unless I'm still asleep, this block of code should say:

             /* abs_added_path had better be a child of abs_target_path
                or something is *really* wrong. */
             SVN_ERR_ASSERT(svn_path_is_child(abs_target_path,
                                              abs_added_path,
                                              iterpool));
             common_ancestor_path = abs_target_path;

- Julian

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=985641
Received on 2008-12-17 12:16:06 CET

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