FilePlugin>>primitiveFileStdioHandles fails incorrectly?

 
Hi Everyone,

FilePlugin>>primitiveFileStdioHandles is responsible for opening stdio
in the VM.  The basic behaviour is to call sqFileStdioHandlesInto()
which does the actual work and returns a mask indicating which streams
were successfully opened (stdin, stdout, stderr).

However primitiveFileStdioHandles regards a mask of 0, i.e. no files
available, as a primitive failure:


sHFAfn ~= 0 ifTrue:
    [(self cCode: ' ((sqInt (*)(void))sHFAfn)()' inSmalltalk: [true]) ifFalse:
        [^interpreterProxy primitiveFailFor: PrimErrUnsupported]].
self cCode: '' inSmalltalk: [fileRecords := Array new: 3].
validMask := self sqFileStdioHandlesInto: fileRecords.
validMask = 0 ifTrue:
    [^interpreterProxy primitiveFailFor: PrimErrUnsupported].


This doesn't cause a problem in squeak since all primitive errors are
handled by simply assuming that the stdio files can't be opened and
ignoring any other errors.

Pharo currently raises an unhandled exception.

On Windows not having stdio available is a normal condition, so
I think the correct behaviour is for the primitive to succeed, but
return an array of 3 nils, i.e. successfully determined that none of the
stdio files are available (this is effectively what happens in the
Squeak image anyway).

It would then mean that a primitive failure can be treated as something
going wrong, rather than the normal (on Windows) situation of no stdio
files being available.

The other (minor) benefit is that at the moment there are two situations
in which the primitive fails with PrimErrUnsupported: sHFAfn ~= 0 and
validMask = 0.  This change would make it easier to interpret the
failure.

Any objections?

Thanks,
Alistair

 
Hi Eliot,

On Wed, 8 Aug 2018 at 21:31, Eliot Miranda <[hidden email]> wrote:
>
>
> Hi Alistair,
>
> On Sun, Aug 5, 2018 at 1:02 PM, Alistair Grant <[hidden email]> wrote:
>>
>>
>> Hi Everyone,
>>
>> FilePlugin>>primitiveFileStdioHandles is responsible for opening stdio
>> in the VM.  The basic behaviour is to call sqFileStdioHandlesInto()
>> which does the actual work and returns a mask indicating which streams
>> were successfully opened (stdin, stdout, stderr).
>>
>> However primitiveFileStdioHandles regards a mask of 0, i.e. no files
>> available, as a primitive failure:
>>
>>
>> sHFAfn ~= 0 ifTrue:
>>     [(self cCode: ' ((sqInt (*)(void))sHFAfn)()' inSmalltalk: [true]) ifFalse:
>>         [^interpreterProxy primitiveFailFor: PrimErrUnsupported]].
>> self cCode: '' inSmalltalk: [fileRecords := Array new: 3].
>> validMask := self sqFileStdioHandlesInto: fileRecords.
>> validMask = 0 ifTrue:
>>     [^interpreterProxy primitiveFailFor: PrimErrUnsupported].
>>
>>
>> This doesn't cause a problem in squeak since all primitive errors are
>> handled by simply assuming that the stdio files can't be opened and
>> ignoring any other errors.
>>
>> Pharo currently raises an unhandled exception.
>>
>> On Windows not having stdio available is a normal condition, so
>> I think the correct behaviour is for the primitive to succeed, but
>> return an array of 3 nils, i.e. successfully determined that none of the
>> stdio files are available (this is effectively what happens in the
>> Squeak image anyway).
>>
>> It would then mean that a primitive failure can be treated as something
>> going wrong, rather than the normal (on Windows) situation of no stdio
>> files being available.
>>
>> The other (minor) benefit is that at the moment there are two situations
>> in which the primitive fails with PrimErrUnsupported: sHFAfn ~= 0 and
>> validMask = 0.  This change would make it easier to interpret the
>> failure.
>>
>> Any objections?
>
>
> No, but I think the right thing to do is to fail if  sqFileStdioHandlesInto answers a value less than zero.  That would;d still allow the subsystem to communicate some serious error, while allowing the system also to report that no streams are available (e.g. when not attached to a console).

Wouldn't it  be better for sqFileStdioHandlesInto to flag the
primitive as failing itself as it could then provide a reason, e.g.
primitiveFailFor or primitiveFailForOSError? (the platform support
file is already doing this in other support routines).


> BTW, is there any image level source code available for accessing the FileAttributesPlugin's primitives?  This is lazy of me, but if there is could you email me a pointer to the code or the code itself asap?  TIA
>
> P.S> I'm specifically looking for code to implement testing directory writability in Squeak.

OK, just a warning in advance: The code (VM and image) works fine on
Linux and Windows.  I didn't know about the Mac file encoding quirks
until recently, so need to change the plugin to work properly on Mac
OS.  But I haven't got the VM to compile on my Mac virtualbox machine,
so the changes are still local to my PC.  This won't affect you unless
you've got non-ascii characters in your file names.  I should have
fixed this by now, but the problems compiling on Mac have frustrated
me, and I've had very little time (personal reasons).

The main repository for the image side file attributes code is at:
https://github.com/akgrant43/pharo/tree/21368-Integrate-FileAttributesPlugin.

If you're not comfortable with git I think figuring out the changes
will be difficult.  I've got a .cs from a slightly earlier revision
which I can email you - probably directly, it's 93Kb.

I'll try and put together a change set which just includes enough code
to get file attributes.

Any other questions, of course please let me know.

Cheers,
Alistair

 

Hi Alistair,

On Thu, Aug 9, 2018 at 1:37 AM, Alistair Grant <[hidden email]> wrote:

 
Hi Eliot,

On Wed, 8 Aug 2018 at 21:31, Eliot Miranda <[hidden email]> wrote:
>
>
> Hi Alistair,
>
> On Sun, Aug 5, 2018 at 1:02 PM, Alistair Grant <[hidden email]> wrote:
>>
>>
>> Hi Everyone,
>>
>> FilePlugin>>primitiveFileStdioHandles is responsible for opening stdio
>> in the VM.  The basic behaviour is to call sqFileStdioHandlesInto()
>> which does the actual work and returns a mask indicating which streams
>> were successfully opened (stdin, stdout, stderr).
>>
>> However primitiveFileStdioHandles regards a mask of 0, i.e. no files
>> available, as a primitive failure:
>>
>>
>> sHFAfn ~= 0 ifTrue:
>>     [(self cCode: ' ((sqInt (*)(void))sHFAfn)()' inSmalltalk: [true]) ifFalse:
>>         [^interpreterProxy primitiveFailFor: PrimErrUnsupported]].
>> self cCode: '' inSmalltalk: [fileRecords := Array new: 3].
>> validMask := self sqFileStdioHandlesInto: fileRecords.
>> validMask = 0 ifTrue:
>>     [^interpreterProxy primitiveFailFor: PrimErrUnsupported].
>>
>>
>> This doesn't cause a problem in squeak since all primitive errors are
>> handled by simply assuming that the stdio files can't be opened and
>> ignoring any other errors.
>>
>> Pharo currently raises an unhandled exception.
>>
>> On Windows not having stdio available is a normal condition, so
>> I think the correct behaviour is for the primitive to succeed, but
>> return an array of 3 nils, i.e. successfully determined that none of the
>> stdio files are available (this is effectively what happens in the
>> Squeak image anyway).
>>
>> It would then mean that a primitive failure can be treated as something
>> going wrong, rather than the normal (on Windows) situation of no stdio
>> files being available.
>>
>> The other (minor) benefit is that at the moment there are two situations
>> in which the primitive fails with PrimErrUnsupported: sHFAfn ~= 0 and
>> validMask = 0.  This change would make it easier to interpret the
>> failure.
>>
>> Any objections?
>
>
> No, but I think the right thing to do is to fail if  sqFileStdioHandlesInto answers a value less than zero.  That would;d still allow the subsystem to communicate some serious error, while allowing the system also to report that no streams are available (e.g. when not attached to a console).

Wouldn't it  be better for sqFileStdioHandlesInto to flag the
primitive as failing itself as it could then provide a reason, e.g.
primitiveFailFor or primitiveFailForOSError? (the platform support
file is already doing this in other support routines).


> BTW, is there any image level source code available for accessing the FileAttributesPlugin's primitives?  This is lazy of me, but if there is could you email me a pointer to the code or the code itself asap?  TIA
>
> P.S> I'm specifically looking for code to implement testing directory writability in Squeak.

OK, just a warning in advance: The code (VM and image) works fine on
Linux and Windows.  I didn't know about the Mac file encoding quirks
until recently, so need to change the plugin to work properly on Mac
OS.  But I haven't got the VM to compile on my Mac virtualbox machine,
so the changes are still local to my PC.  This won't affect you unless
you've got non-ascii characters in your file names.  I should have
fixed this by now, but the problems compiling on Mac have frustrated
me, and I've had very little time (personal reasons).

I develop on the Mac so feel free to email me a change set and tests (e.g. just doit in a workspace) and IO can test for you.

 

The main repository for the image side file attributes code is at:
https://github.com/akgrant43/pharo/tree/21368-Integrate-FileAttributesPlugin.

OK, I'll see if I can grab this wen I have time. Today is taken :-)

 

If you're not comfortable with git I think figuring out the changes
will be difficult.  I've got a .cs from a slightly earlier revision
which I can email you - probably directly, it's 93Kb.

I'll try and put together a change set which just includes enough code
to get file attributes.

I've got myself going.  I have the R_OK, W_OK, E_OK accessors in place so don't need source code.  I do suggest however that primitiveFileAttribute would be nicer to use if it took its arguments in reverse order.  i.e.

    attribute: attributeNumber forPath: path

is less clumsy than

    attributeForPath: path numbered: attributeNumber

Any other questions, of course please let me know.

Cheers,
Alistair

_,,,^..^,,,_

best, Eliot