-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Thanks Julian fixed in r34760(revert of r34756) and r34762.
With regards
Kamesh Jayachandran
Julian Foad wrote:
> Kamesh Jayachandran wrote:
>> 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."
>
> Oops, yes, that's what I meant. Sorry. I'll say it a different way as
> well:
>
> You are now asserting only that ABS_ADDED_PATH and ABS_TARGET_PATH have
> some common ancestor. Previously you were asserting that ABS_ADDED_PATH
> was a child of ABS_TARGET_PATH. Those are different assertions.
>
>>> 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).
>
> Sure, yes, both of those cases fail. But if the two paths have a common
> ancestor but are NOT parent and child:
>
> abs_target_path='/a/z'
> abs_added_path='/a/b/c'
>
> common_anc=svn_path_get_longest_ancestor(abs_added_path,
> abs_target_path,iterpool) returns "/a" so
> SVN_ERR_ASSERT(*common_anc) passes, which is wrong.
>
> The original check:
>
> SVN_ERR_ASSERT(svn_path_is_child(abs_target_path,
> abs_added_path,
> iterpool));
>
> would have failed, because '/a/b/c' is not a child of '/a/z'.
>
> - Julian
>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFJSQAC3WHvyO0YTCwRAqx2AJ4og17mK3rbAa7AHKlrr5o7W3/gIACgpvtB
XMpS+Iu+e2IxLkXwogZq9f4=
=zXbC
-----END PGP SIGNATURE-----
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=985694
Received on 2008-12-17 14:34:00 CET