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

[Michael Van Canneyt]

On Sun, 16 Jun 2013, Michalis Kamburelis wrote:

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

The above implementation should not be changed, it is Delphi compatible:
TStream.ReadBuffer works on the assumption that Read will always return 
the amount of bytes requested if they are available.

So, if you want to make changes, there is no reason why TStream.Read should not benefit 
from 'trying harder', it could try harder to actually get the requested number of 
bytes from the underlying low-level implementation. Some TStream descendents already 
work that way (compression, hash, encoding), others don't.

This in turn means that affected TStream descendents such as TFileStream or THandleStream 
should try harder to read the requested number of bytes, taking into account the
specifics of the underlying file/socket/whatever protocol.

Michael.