[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: Kamesh Jayachandran <kamesh_at_collab.net>
Date: Wed, 17 Dec 2008 17:33:12 +0530

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Julian Foad wrote:
> 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.

My head spins a bit when I think about it many times :),

Hope you meant,
"Previously you were asserting that *ABS_TARGET_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
>

I think r34756 is another way of doing the same thing(check of paths).

Let me put it with a data I have in my mind, if you disagree please let
me know with data so that I can understand easily.

let us say
 abs_target_path='/a'
 abs_added_path='/b/c'

common_anc=svn_path_get_longest_ancestor(abs_added_path,
                                abs_target_path,iterpool) returns "" so
SVN_ERR_ASSERT(*common_anc) fails in assertion.

svn_path_is_child(abs_target_path,abs_added_path,iterpool) returns NULL

I *think* both first SVN_ERR_ASSERT() and
SVN_ERR_ASSERT(svn_path_is_child(...)) yields same results(assertion
failure).

With regards
Kamesh Jayachandran
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFJSOqA3WHvyO0YTCwRAqWzAJ9YYHTRigzSClrE271yoqT/f7+JeQCghyD0
U71/0zTeC+6r7FMUXvwVdVU=
=CqSs
-----END PGP SIGNATURE-----

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

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