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

[Alistair Grant]

On 26 March 2018 at 23:51, David T. Lewis <lewis at 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:
>>
>> 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.
>>
>> 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