[merge] Assert that readv() is returning the right data

John Arbash Meinel john at arbash-meinel.com
Tue Sep 12 17:45:05 BST 2006


I realized we had a potential bug in Transport, where if a read()
returned a short amount of data, it could cause bogus results elsewhere.

So I investigated updating the Transport.readv() api, so that if a short
read occurs, it raises an ShortReadvError, rather than silently failing.

However, it turns out that we already have 'assert' sprinkled elsewhere
throughout the code. And more importantly 'paramiko' asserts the length
of the read. So while I can fix our Transports so that they raise a
specific exception, I can't change paramiko to not die with an assert.

So the attached patch just fixes Transport._seek_and_read() so that it
at least asserts the data length is correct.

The other fix I can think of, is to write a test case, and make sure
that either ShortReadvError or AssertionError is raised, but I don't
really like catching AssertionError.

Assert also is not the best test for this stuff, because it is data
returned from a remote location, and when running with -O no asserts are
evaluated. So we could get silent corruption.

Any suggestions on the best fix?

John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: short_read.patch
Type: text/x-patch
Size: 1449 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060912/fb2157c0/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060912/fb2157c0/attachment.pgp 


More information about the bazaar mailing list