You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Joe Swatosh <jo...@gmail.com> on 2008/10/12 23:10:22 UTC
Re: svn commit: r33528 - in trunk/subversion: include libsvn_delta libsvn_wc
Hi Greg,
On Tue, Oct 7, 2008 at 2:16 PM, <gs...@tigris.org> wrote:
> Author: gstein
> Date: Tue Oct 7 14:16:03 2008
> New Revision: 33528
>
> Log:
> Combine the svn_txdelta() and svn_txdelta_send_txstream() functions into a
> single function (since these always come in pairs). In addition, upgrade
> the API style to latest: use svn_checksum, use two pools, and add a cancel
> function.
>
> Add an example use of it in svn_wc_transmit_text_deltas2(). This function
> has some crazy checksum stuff in it, so we're cleaning it all up through the
> new txdelta routine.
>
> * subversion/include/svn_delta.h:
> (svn_txdelta_run): new declaration
>
> * subversion/libsvn_delta/text_delta.c:
> (struct txdelta_baton): toss md5 context, add svn_checksum context.
> add a result pool for the final chcksum.
> (txdelta_next_window): switch to svn_checksum
> (txdelta_md5_digest): get digest from txdelta_baton.checksum->digest
> (svn_txdelta_run): new function
> (svn_txdelta): update initialization of baton.
>
The ruby bindings started having a test failure after this commit.
Normally I just try to figure out and fix what is going on in the
bindings, but sometimes the bindings tests are sort of a canary in a
coalmine for other issues.
Bottom line this test:
def test_txdelta_window
s = ("a\nb\nc\nd\ne" + "\n" * 100) * 1000
t = ("a\nb\nX\nd\ne" + "\n" * 100) * 1000
source = StringIO.new(s)
target = StringIO.new(t)
stream = Svn::Delta::TextDeltaStream.new(source, target)
assert_nil(stream.md5_digest)
_wrap_assertion do
stream.each do |window|
window.ops.each do |op|
op_size = op.offset + op.length
case op.action_code
when Svn::Delta::TXDELTA_SOURCE
assert_operator(op_size, :<=, window.sview_len)
when Svn::Delta::TXDELTA_NEW
assert_operator(op_size, :<=, window.new_data.length)
when Svn::Delta::TXDELTA_TARGET
assert_operator(op_size, :<=, window.tview_len)
else
flunk
end
end
end
end
assert_equal(MD5.new(t).hexdigest, stream.md5_digest) # this assertion fails
end
Now gives this result:
1) Failure:
test_txdelta_window(SvnDeltaTest)
[D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/test_delta.rb:47]:
<"f6fd44565e14c6e44b35292719deb77e"> expected but was
<"f6fd44565e14c6e44434333433333334">.
line 47 is the marked assertion. So it appears that
svn_txdelta_md5_digest is returning a different value than it used to.
Not sure if its significant, but I was interested in your opinion
about if there is a better way to test this or if the problem is the
changed return value.
Thanks,
--
Joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33528 - in trunk/subversion: include libsvn_delta libsvn_wc
Posted by Greg Stein <gs...@gmail.com>.
On Sun, Oct 12, 2008 at 4:10 PM, Joe Swatosh <jo...@gmail.com> wrote:
>...
> def test_txdelta_window
> s = ("a\nb\nc\nd\ne" + "\n" * 100) * 1000
> t = ("a\nb\nX\nd\ne" + "\n" * 100) * 1000
> source = StringIO.new(s)
> target = StringIO.new(t)
> stream = Svn::Delta::TextDeltaStream.new(source, target)
> assert_nil(stream.md5_digest)
> _wrap_assertion do
> stream.each do |window|
> window.ops.each do |op|
> op_size = op.offset + op.length
> case op.action_code
> when Svn::Delta::TXDELTA_SOURCE
> assert_operator(op_size, :<=, window.sview_len)
> when Svn::Delta::TXDELTA_NEW
> assert_operator(op_size, :<=, window.new_data.length)
> when Svn::Delta::TXDELTA_TARGET
> assert_operator(op_size, :<=, window.tview_len)
> else
> flunk
> end
> end
> end
> end
> assert_equal(MD5.new(t).hexdigest, stream.md5_digest) # this assertion fails
> end
>
> Now gives this result:
>
> 1) Failure:
> test_txdelta_window(SvnDeltaTest)
> [D:/SVN/src-trunk/subversion/bindings/swig/ruby/test/test_delta.rb:47]:
> <"f6fd44565e14c6e44b35292719deb77e"> expected but was
> <"f6fd44565e14c6e44434333433333334">.
>
> line 47 is the marked assertion. So it appears that
> svn_txdelta_md5_digest is returning a different value than it used to.
>
> Not sure if its significant, but I was interested in your opinion
> about if there is a better way to test this or if the problem is the
> changed return value.
Damn. That shouldn't have changed.
Let me take a look...
Thx,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33528 - in trunk/subversion: include libsvn_delta libsvn_wc
Posted by Jeremy Whitlock <jc...@gmail.com>.
> This may also fix the memory corruption that Jeremy was seeing.
Mine still exists.
--
Take care,
Jeremy Whitlock
http://www.thoughtspark.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33528 - in trunk/subversion: include libsvn_delta libsvn_wc
Posted by Joe Swatosh <jo...@gmail.com>.
On Mon, Oct 13, 2008 at 5:02 AM, Greg Stein <gs...@gmail.com> wrote:
> On Sun, Oct 12, 2008 at 4:10 PM, Joe Swatosh <jo...@gmail.com> wrote:
>>...
>> line 47 is the marked assertion. So it appears that
>> svn_txdelta_md5_digest is returning a different value than it used to.
>>
>> Not sure if its significant, but I was interested in your opinion
>> about if there is a better way to test this or if the problem is the
>> changed return value.
>
> I just added a new test: subversion/tests/libsvn_delta/window-test.c.
> That appears to show the digest is being returned properly.
>
> Without further pointers, I'm going to assume that there is some
> memory corruption in how Ruby transfers that digest into a Ruby value.
> Note that the first half of the digest is the same as the expected
> value, but the last half has 0x44 ('D'), then a combo of 0x33 ('3')
> and 0x34 ('4') values. Given the way that MD5 works, I am *extremely*
> suspicious that a hash was able to match the first eight bytes.
>
> Oh. And I think that I see the problem. I added a "result_pool" into
> the txstream baton for allocation of the checksum, but didn't actually
> use it! I've just fixed that and committed (r33613). Please try your
> test again.
>
Ta, Greg. Ruby bindings tests are green again at r33613 (with a few
patches for other issues that I hope to be committing soon).
Thanks again,
--
Joe Swatosh
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33528 - in trunk/subversion: include libsvn_delta libsvn_wc
Posted by Greg Stein <gs...@gmail.com>.
On Sun, Oct 12, 2008 at 4:10 PM, Joe Swatosh <jo...@gmail.com> wrote:
>...
> line 47 is the marked assertion. So it appears that
> svn_txdelta_md5_digest is returning a different value than it used to.
>
> Not sure if its significant, but I was interested in your opinion
> about if there is a better way to test this or if the problem is the
> changed return value.
I just added a new test: subversion/tests/libsvn_delta/window-test.c.
That appears to show the digest is being returned properly.
Without further pointers, I'm going to assume that there is some
memory corruption in how Ruby transfers that digest into a Ruby value.
Note that the first half of the digest is the same as the expected
value, but the last half has 0x44 ('D'), then a combo of 0x33 ('3')
and 0x34 ('4') values. Given the way that MD5 works, I am *extremely*
suspicious that a hash was able to match the first eight bytes.
Oh. And I think that I see the problem. I added a "result_pool" into
the txstream baton for allocation of the checksum, but didn't actually
use it! I've just fixed that and committed (r33613). Please try your
test again.
This may also fix the memory corruption that Jeremy was seeing.
Cheers,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org