Bug in SmartMedium._get_line()
robey at lag.net
Tue Jun 12 18:24:49 BST 2007
On 5 Jun 2007, at 18:29, Robert Collins wrote:
> On Mon, 2007-05-21 at 13:04 +0200, John Arbash Meinel wrote:
>> Hey, I thought I found a bug in SmartMedium._get_line()
>> Specifically, it is doing:
>> line = ''
>> while not line or line[-1] != '\n':
^ I think this is what's bothering John.
>> new_char = self._get_bytes(1)
>> line += new_char
>> if new_char == '':
>> # Ran out of bytes before receiving a complete line.
>> return line
>> However, the SmartServerSocketStreamMedium._get_bytes is defined as:
>> def _get_bytes(self, desired_count):
>> # We ignore the desired_count because on sockets it's more
>> efficient to
>> # read 4k at a time.
>> return self.socket.recv(4096)
>> So I may be mis-reading what is calling what, but it sure looks like
>> "_get_lines" is explicitly assuming no more than 1 character will be
>> returned. And at a minimum it is only working when the socket returns
>> exactly a single line.
> No, You're misreading - + appends the entire returned bytestring. Its
> saying that it needs 1 or more characters - dont bother returning with
> less than 1 unless its EOF.
> We can make this more clear, obviously.
If _get_bytes(1) can return more than 1 byte, it might return
something like '\nmore' and then line[-1] won't be '\n'.
It would probably be better to make _get_bytes(N) promise to return
no more than N bytes, possibly buffering more data internally. This
is how other socket APIs work, so would be less surprising behavior.
More information about the bazaar