Hi Levente,
Right, my nice detailed description is all about the symptoms and not the actual issue. Sorry about that.
You're also correct that a new Pharo stream (ZnBufferedReadStream) had a bug in it, which is in the process of being fixed.
The actual issue is the value returned by #atEnd, which is returning true as you suggested:
| outputs |
outputs := OrderedCollection new. StandardFileStream readOnlyFileNamed: '/dev/urandom' do: [ :file | outputs add: (file binary; next: 8). outputs add: file atEnd. outputs add: (file binary; next: 8). outputs add: file atEnd. ]. outputs " an OrderedCollection( #[80 31 245 49 20 97 163 76] true #[207 210 106 185 26 86 15 49] true)"
The proposed modification means that #atEnd returns the correct value for non-regular files like the example above (false), while still maintaining the existing behaviour for regular files and stdio streams.
Hopefully that is clearer, Alistair
On 26 March 2018 at 18:16, Levente Uzonyi leves@caesar.elte.hu wrote:
FilePlugin currently splits files in to two groups: 1) Stdio streams and 2) everything else.
To test for the end of file, FilePlugin>>primitiveFileAtEnd:
- Uses feof() for stdio streams.
- Compares the current position to the file size for everything else.
This returns the expected results for regular files, but fails for non-regular files, e.g.:
(FileSystem / 'dev' / 'urandom') binaryReadStream next: 8.
returns an empty array.
The FileStream hierachy in Squeak has no issue with the current implementation, because the code will call the primitive to read from the file even if #atEnd returns true (which conforms to what you described as expected usage). E.g.
StandardFileStream readOnlyFileNamed: '/dev/urandom' do: [ :file | file binary; next: 8 ]
will return #[192 229 133 67 128 216 51 114].
On Unix, the proper way to check is to read past the end of the file and then call feof() to confirm that it is in fact at the end.
Pharo (and I assume, Squeak) has plenty of code that assumes that a file position >= the file size means that the file is at the end, and as stated above this generally works for regular files.
It seems that the n+1th stream implementation that was incorporated into Pharo is not compatible with the rest of the stream libraries (the original Stream hierarchy - as shown above, Nile, Xtreams, and whatever else is out there I'm not aware of). If it were, then this issue would have arisen long before. So, if I were you, I would first check if the new implementation in Pharo is correct.
Levente
This patch modifies FilePlugin>>primitiveFileAtEnd to:
a) Keep the current behaviour of using the file position test for regular files. b) Keep the current behaviour of using feof() for stdio streams. c) Use feof() for non-regular files, e.g. /dev/urandom.
This allows existing code to continue to function, and allows non-regular files to be tested correctly.
After applying the patch, the example code above answers the expected result, e.g.:
(FileSystem / 'dev' / 'urandom') binaryReadStream next: 8. " #[179 136 227 226 28 147 197 125]"
On Windows, as far as I can tell, all files are regular, and the position test is used regularly, so no change is requried.
https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/232 contains the changes. The CI builds failed, but for different reasons to do with sista.
If I don't hear any dissenting opinion in a few days I'll integrate this in.
Cheers, Alistair