[fpc-pascal] TStream.ReadBuffer doesn't try hard enough to read the requested bytes

[Michalis Kamburelis]

Hi,

I'm often reading fixed-size structures using TStream.ReadBuffer, 
working under the assumption that it will fail only if it's not possible 
to read the requested amount of bytes (that is, if the stream ended 
prematurely). This is actually the documented usage, see 
http://www.freepascal.org/docs-html/rtl/classes/tstream.readbuffer.html 
and see many usage examples of ReadBuffer in FPC, Lazarus etc. sources. 
I'm using various stream classes in my code (TFileStream, TGZFileStream, 
TMemoryStream, TBase64DecodingStream...) and I always depend that 
ReadBuffer will work correctly for all streams.

But in some hardly-reproducible cases sometimes ReadBuffer fails even 
though I know I have more bytes. Looking at the sources, I was surprised 
to see this ReadBuffer implementation (in rtl/objpas/classes/streams.inc ):

procedure TStream.ReadBuffer(var Buffer; Count: Longint);
     begin
        if Read(Buffer,Count)<Count then
          Raise EReadError.Create(SReadError);
     end;

This is quite incorrect, IMHO. The TStream.Read implementations of 
various streams can sometimes return less Count than requested, and 
that's Ok, and that doesn't mean that stream ended. Stream ended only 
when TStream.Read returns exactly zero.

Which means that ReadBuffer should retry calling Read, in a loop, trying 
to finish the reading. ReadBuffer should raise an exception only when 
Read returns zero (before we can gather all the Count bytes).

Right now, the way I see it, a lot of the code using ReadBuffer works 
purely by accident. At least if it's supposed to work with any TStream 
descendant.

For example: on Linux, TFileStream.Read calls fpRead which is a system 
call, and it definitely can return less than requested amount of bytes, 
and it doesn't mean that stream ended. (See e.g. Libc docs: 
http://www.gnu.org/software/libc/manual/html_mono/libc.html#I_002fO-Primitives 
: "If read returns at least one character, there is no way you can tell 
whether end-of-file was reached. But if you did reach the end, the next 
read will return zero. ") So if you use TFileStream with ReadBuffer, 
your code is working purely by accident right now...

If you agree, I can provide a patch to ReadBuffer (and similar to 
WriteBuffer).

Michalis