[Vm-dev] Proposed mod to FilePlugin>>primitiveFileAtEnd for non-regular files

[Levente Uzonyi]

> 
> FilePlugin currently splits files in to two groups: 1) Stdio streams and
> 2) everything else.
>
> To test for the end of file, FilePlugin>>primitiveFileAtEnd:
>
> 1) Uses feof() for stdio streams.
> 2) 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