On 26 March 2018 at 23:51, David T. Lewis lewis@mail.msen.com wrote:
FYI, OSProcess provides this:
OSProcessAccessor>>isAtEndOfFile: anIOHandle "Answer whether the file represented by anIOHandle is at end of file, as determined by a call to feof(). This is different from StandardFileStream>>primAtEnd: which answers true if the file pointer is at the end of the file, but which does not call feof() to determine that an end of file condition has occurred. The difference is significant if aSqFileStruct represents a pipe or a device file, which may not be positionable in the sense of a conventional disk file."
And the primitive for Unix is:
UnixOSProcessPlugin>>primitiveTestEndOfFileFlag "Take a struct SQFile from the stack, and call feof(3) to determine if the file has reached end of file. The flag is set only by a previous read operation, so end of file is not detected until an actual EOF condition has been detected by a read attempt."
Typical usage is a file stream on an OS pipe stream, in which case OSProcess makes a distiction between reading to end of file (current available data in a stream) versus reading all available data until the remote end of the pipe is closed (hence all of the data that is or ever will be available on that stream).
In my opinion, the distinction between "stdio streams" and "everything else" is a mis-feature,
+1
and it presumably entered the FilePlugin to accommodate Windows. The console streams on Windows are quite fundamentally different from other streams, and have pre-assigned handle numbers to identify them. This is not the case for Unix-based systems, which attempt to treat IO streams as similarly as possible, and which are free to dup() the handles at will.
The general idea is that stdin, stdout, and stderr should behave as reasonably as possible regardless of whether they are attached to a positional stream on a disk file, or to a pipe connected to some other process, or to some sort of special file. Or to say it another way, The stdin/stdout/stderr streams might be connected to things that are positionable or non-positionable, and you should not try to guess which behavior is appropriate based on whether that stream happens to be labeled as a "stdio stream".
Right. Given that the primitive for opening/connecting to a file by file descriptor is now in to the VM, combined with this proposed patch it's possible that on Unix we could get rid of the special stdio stream handling altogether (I haven't gone through all the code to check). On Windows it should be possible to move the special handling out of the VM and in to the image, but again, that is obviously more work.
Cheers, Alistair
On Mon, Mar 26, 2018 at 05:32:15PM +0200, Alistair Grant 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.
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.
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