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

[Ewald]

On 16 Jun 2013, at 03:18, Michalis Kamburelis wrote:

> Ewald wrote:
>> And what with non-blocking pipes pipes? Wait for a *some* period
>> until you get all data? It is up to the programmer to do this INHO.
> 
> If you want to get partial data (instead of waiting until all requested data is available) you should just use TStream.Read instead of TStream.ReadBuffer. Then it's indeed up to the programmer to handle this.
> 

Sorry, I misread your mail last night (it was getting late I believe), I thought you said `TStream.Read` instead of `TStream.ReadBuffer`. In this case you are probably right.

> 
>> Also, if you would enforce this behaviour on all kinds of streams,
>> you might (`will`, actually) get unexpected performance drops...
> 
> I don't see why you would get any performance drop. If you're lucky and a single Read call is all you need, then the speed is the same as current implementation, since the 1st check "if Read(Buffer,Count)<Count then" will return false and we will not call Read again.

Never mind, my brain was still thinking on the wrong call here. Sorry for the noise there :-)


>> 
>> But if you work with a blocking fd (which, for example, a TFileStream
>> uses IIRC) you will always get your data. If there is more data to be
>> read(), but it would take time, it simply takes time. The only case
>> where zero is returned, is IIRC (like you quoted) when the end of the
>> `fd` (e.g. pipe, socket, file) is reached on *blocking
>> filedescriptors*.
> 
> Yes, zero is returned only if stream ends. But a result that is non-zero, but still less than requested Count, doesn't tell you if the stream ends. The whole point is whether ReadBuffer should take this into account.

If the `ReadBuffer` call uses `Read` to read the data (which would be rather obvious) than yes, it would need to take this into account as it cannot know which subsystem `Read` uses.


> 
>> FYI: I've never had the problem of a `partial read` (e.g.
>> SomeStream.Read(50) returning 36 or something) on linux, osx and
>> windows; so perhaps you have exposed some bug in an obscure TStream
>> descendant?
>> 
> 
> Reproducing this behavior proved to be difficult, but I definitely observed it with TFileStream on Linux.

Did it got interrupted by a signal then? Or perhaps the `Count` argument of `ReadBuffer` was greater than SSIZE_MAX? Either way, as ReadBuffer gives no information about the number of bytes actually read, it should indeed `try harder`.

> 
> And, even if it would accidentally work for some streams, that's not a solution. Like I mentioned, TStream.ReadBuffer is non-virtual and must be suitable for every TStream.
> 

The question that arises here is whether all the `ReadByte`, `ReadWord`, `WriteByte`, `WriteWord`, `WriteBuffer`, ... should also `try harder`? Same issue there: no data is returned about the actual number of bytes read. Anyway, I leave that to someone who knows the internals of these classes.

--
Ewald