Bug in SmartMedium._get_line()

[Robey Pointer]

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.
>>                  break
>>          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.

robey