[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