[RFC/merge] Assert that readv() is returning the right data
Vincent Ladeuil
v.ladeuil at alplog.fr
Wed Sep 13 08:53:07 BST 2006
>>>>> "jam" == John Arbash Meinel <john at arbash-meinel.com> writes:
jam> John Arbash Meinel wrote:
>> 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.
>>
jam> ...
>> 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.
>>
jam> Attached is an alternative that we can use if we
jam> prefer. It also finds a bug in 'SFTPTransport.readv()'
jam> when the seek is outside of the file. (The inner loop
jam> returns no data, so the outer loop returns no texts, but
jam> no errors are ever raised).
jam> Only SFTP fails with AssertionError, so it might be
jam> reasonable to change the test to only allow
jam> AssertionError in the case that the transport is sftp.
jam> John
jam> =:->
jam> # Bazaar revision bundle v0.8
jam> #
jam> # message:
jam> # Force all transports to raise ShortReadvError if they can
jam> # committer: John Arbash Meinel <john at arbash-meinel.com>
jam> # date: Tue 2006-09-12 12:03:28.296000004 -0500
jam> === modified file bzrlib/errors.py
jam> --- bzrlib/errors.py
jam> +++ bzrlib/errors.py
jam> @@ -285,6 +285,17 @@
jam> PathError.__init__(self, url, extra=extra)
jam> +class ShortReadvError(PathError):
jam> + """readv() could not read the %(length)s bytes at %(offset)s for %(path)s%(extra)s"""
But how much did it read then ?
<snip/>
jam> === modified file bzrlib/transport/http/__init__.py
jam> --- bzrlib/transport/http/__init__.py
jam> +++ bzrlib/transport/http/__init__.py
jam> @@ -254,7 +254,9 @@
jam> f.seek(start, (start < 0) and 2 or 0)
jam> start = f.tell()
jam> data = f.read(size)
jam> - assert len(data) == size
jam> + if len(data) != size:
jam> + raise errors.ShortReadvError(relpath, start, size,
jam> + extra='Only read %s bytes' % (len(data),))
jam> yield start, data
Either delete the 'Only' or do len(data) < size. Or better,
reports how many bytes you read.
Vincent
More information about the bazaar
mailing list