Fwd: Merging FilesAttributesPlugin

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Fwd: Merging FilesAttributesPlugin

Clément Béra
 
Hi,

What is the status of the FilesAttributesPlugin ? I would like to make it available by default on pharo VMs.

Questions:

1) Is it already merged with another plugin ?

2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?

3) What is the right way to generate a plugin ? I use this:

(VMMaker
makerFor: StackInterpreter
and: nil
with: #()
to: (FileDirectory default pathFromURI: '../src')
platformDir: (FileDirectory default pathFromURI: '../platforms')
including: #(FileAttributesPlugin)
) generateExternalPlugin: FileAttributesPlugin

Is that correct ?
It seems it generates only the C file but no header file.

Thanks,
 
--
Clément Béra
Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq


Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

alistairgrant
 
Hi Clément,

On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>
> Hi,
>
> What is the status of the FilesAttributesPlugin ?

I'm hoping that it will be integrated soon.


> I would like to make it available by default on pharo VMs.

Yes, please. :-)


> Questions:
>
> 1) Is it already merged with another plugin ?

I'm not sure that I understand the question.  FileAttributesPlugin
works along side FilePlugin (which is obviously already part of the
pharo VM).


> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?

That's my guess, but someone knowledgable needs to answer this.


> 3) What is the right way to generate a plugin ? I use this:
>
> (VMMaker
> makerFor: StackInterpreter
> and: nil
> with: #()
> to: (FileDirectory default pathFromURI: '../src')
> platformDir: (FileDirectory default pathFromURI: '../platforms')
> including: #(FileAttributesPlugin)
> ) generateExternalPlugin: FileAttributesPlugin
>
> Is that correct ?
> It seems it generates only the C file but no header file.

There isn't a FileAttributesPlugin.h.  It does need the platform support file:

platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin

which is already part of opensmalltalk-vm.

I've always used VMMakerTool to make the plugin, but if it's
generating the .c file it's probably correct.


My plan was that once the plugin was added to VMMaker-Plugins that I
would test it again on linux and windows, and then ask for the
appropriate plugins.int to be updated.


Thanks!
Alistair


> Thanks,
>
> --
> Clément Béra
> https://clementbera.wordpress.com/
> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

Eliot Miranda-2
 
Hi Clément, Hi Alistair,

    FileAttrbutesPlugin is not yet ready for use.  The main problem is that it neither fails properly nor communicates error codes back properly.  Basically FileAttrbutesPlugin confuses failing because it gets invalid arguments with reporting error codes due to operating system level errors.  For the FileAttrbutesPlugin's primitives to work in Spur they must fail in the traditional way when given invalid arguments.  This is because the Spur VM will check when a primitive fails if there are any forwarders in the primitive's parameters, and if so, follow (fix up) the forwarders and retry the primitive.  

Next, communication no os errors back through the primitives results is acceptable, but a better way would be to use the new #'operating system error' primitive failure code mechanism.  Here, provided a suitable two slot prototype is installed in the primitiveErrorTable (see Squeak trunk) then a plugin can do
        interpreterProxy primitiveFailForOSError(signedSixtyFourBitErrorCode)
and the VM will clone the error object whose first slot should be #'operating system error' and whose second slot will be the code.

This gives us a clean way of communicating errors back to a primitive's method body in the traditional way.

I do apologize if I hadn't made this clear earlier.  But AFAIA there is significant work to do before the plugin is ready for integration.

_,,,^..^,,,_ (phone)

> On Dec 22, 2017, at 2:48 AM, Alistair Grant <[hidden email]> wrote:
>
>
> Hi Clément,
>
>> On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>>
>> Hi,
>>
>> What is the status of the FilesAttributesPlugin ?
>
> I'm hoping that it will be integrated soon.
>
>
>> I would like to make it available by default on pharo VMs.
>
> Yes, please. :-)
>
>
>> Questions:
>>
>> 1) Is it already merged with another plugin ?
>
> I'm not sure that I understand the question.  FileAttributesPlugin
> works along side FilePlugin (which is obviously already part of the
> pharo VM).
>
>
>> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
>
> That's my guess, but someone knowledgable needs to answer this.
>
>
>> 3) What is the right way to generate a plugin ? I use this:
>>
>> (VMMaker
>> makerFor: StackInterpreter
>> and: nil
>> with: #()
>> to: (FileDirectory default pathFromURI: '../src')
>> platformDir: (FileDirectory default pathFromURI: '../platforms')
>> including: #(FileAttributesPlugin)
>> ) generateExternalPlugin: FileAttributesPlugin
>>
>> Is that correct ?
>> It seems it generates only the C file but no header file.
>
> There isn't a FileAttributesPlugin.h.  It does need the platform support file:
>
> platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin
>
> which is already part of opensmalltalk-vm.
>
> I've always used VMMakerTool to make the plugin, but if it's
> generating the .c file it's probably correct.
>
>
> My plan was that once the plugin was added to VMMaker-Plugins that I
> would test it again on linux and windows, and then ask for the
> appropriate plugins.int to be updated.
>
>
> Thanks!
> Alistair
>
>
>> Thanks,
>>
>> --
>> Clément Béra
>> https://clementbera.wordpress.com/
>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

Eliot Miranda-2
In reply to this post by Clément Béra
 
Hi Clément,

On Dec 22, 2017, at 2:29 AM, Clément Bera <[hidden email]> wrote:

Hi,

What is the status of the FilesAttributesPlugin ? I would like to make it available by default on pharo VMs.

Questions:

1) Is it already merged with another plugin ?

2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?

3) What is the right way to generate a plugin ? I use this:

(VMMaker
makerFor: StackInterpreter
and: nil
with: #()
to: (FileDirectory default pathFromURI: '../src')
platformDir: (FileDirectory default pathFromURI: '../platforms')
including: #(FileAttributesPlugin)
) generateExternalPlugin: FileAttributesPlugin

That looks right to me.  Header files have to be written by hand.  They define the internal interface the plugin uses, not the external interface of the plugin.

You can see how plugins are generated "officially" by following what generateVMPlugins does.  It is also sent by generateAllConfigurationsUnderVersionControl which is the master generator.

For convenience I open a VMMakerTool to build plugins, with
    Interpreter class name = StackInterpreter
    Path to platforms source = as appropriate for your directories, e.g. ../platforms
    Platform name = Cross
    Path to generated sources = as appropriate for your directories, e.g. ../src 
Drag plugins to the right hand list and use the pop up menu to generate the one you select.


Is that correct ?
It seems it generates only the C file but no header file.

Thanks,
 
--
Clément Béra
Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq


Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

alistairgrant
In reply to this post by Eliot Miranda-2
 
Hi Eliot,

On 22 December 2017 at 17:09, Eliot Miranda <[hidden email]> wrote:
>
> Hi Clément, Hi Alistair,
>
>     FileAttrbutesPlugin is not yet ready for use.  The main problem is that it neither fails properly nor communicates error codes back properly.  Basically FileAttrbutesPlugin confuses failing because it gets invalid arguments with reporting error codes due to operating system level errors.  For the FileAttrbutesPlugin's primitives to work in Spur they must fail in the traditional way when given invalid arguments.  This is because the Spur VM will check when a primitive fails if there are any forwarders in the primitive's parameters, and if so, follow (fix up) the forwarders and retry the primitive.

Is this the feedback you gave me on 2 September, or something different?

Back then, you wrote:
>The change that is required to error handling:
> If a primitive fails due to argument marshaling it must use the primitiveFailedWith: mechanism and report the error using one of the error codes.  This is because Spur uses lazy forwarding to implement become:, pin, et al.  The lazy forwarding mechanism follows forwarders in the VM when they are encountered.  For sends this is trivial; any send to a forwarder fails and so the failure side of message lookup follows forwarders and retries the send.  Similarly for the arguments of primitives, when a primitive fails the VM searches for forwarders in the receiver and arguments to a specific pre-computed depth, following any forwarders it finds.  If any forwarders are found the primitive is retried after following all the forwarders found.


My understanding is that this has already been done.  If I've
misunderstood, my apologies.


> Next, communication no os errors back through the primitives results is acceptable, but a better way would be to use the new #'operating system error' primitive failure code mechanism.  Here, provided a suitable two slot prototype is installed in the primitiveErrorTable (see Squeak trunk) then a plugin can do
>         interpreterProxy primitiveFailForOSError(signedSixtyFourBitErrorCode)
> and the VM will clone the error object whose first slot should be #'operating system error' and whose second slot will be the code.
>
> This gives us a clean way of communicating errors back to a primitive's method body in the traditional way.

At the moment I'm passing back an error number generated by the
plugin, not an OS error number returned by a system call, but I assume
that the VM doesn't really care.

Do you have an example where this is being used in a plugin?


Cheers,
Alistair




> I do apologize if I hadn't made this clear earlier.  But AFAIA there is significant work to do before the plugin is ready for integration.
>
> _,,,^..^,,,_ (phone)
>
>> On Dec 22, 2017, at 2:48 AM, Alistair Grant <[hidden email]> wrote:
>>
>>
>> Hi Clément,
>>
>>> On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>>>
>>> Hi,
>>>
>>> What is the status of the FilesAttributesPlugin ?
>>
>> I'm hoping that it will be integrated soon.
>>
>>
>>> I would like to make it available by default on pharo VMs.
>>
>> Yes, please. :-)
>>
>>
>>> Questions:
>>>
>>> 1) Is it already merged with another plugin ?
>>
>> I'm not sure that I understand the question.  FileAttributesPlugin
>> works along side FilePlugin (which is obviously already part of the
>> pharo VM).
>>
>>
>>> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
>>
>> That's my guess, but someone knowledgable needs to answer this.
>>
>>
>>> 3) What is the right way to generate a plugin ? I use this:
>>>
>>> (VMMaker
>>> makerFor: StackInterpreter
>>> and: nil
>>> with: #()
>>> to: (FileDirectory default pathFromURI: '../src')
>>> platformDir: (FileDirectory default pathFromURI: '../platforms')
>>> including: #(FileAttributesPlugin)
>>> ) generateExternalPlugin: FileAttributesPlugin
>>>
>>> Is that correct ?
>>> It seems it generates only the C file but no header file.
>>
>> There isn't a FileAttributesPlugin.h.  It does need the platform support file:
>>
>> platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin
>>
>> which is already part of opensmalltalk-vm.
>>
>> I've always used VMMakerTool to make the plugin, but if it's
>> generating the .c file it's probably correct.
>>
>>
>> My plan was that once the plugin was added to VMMaker-Plugins that I
>> would test it again on linux and windows, and then ask for the
>> appropriate plugins.int to be updated.
>>
>>
>> Thanks!
>> Alistair
>>
>>
>>> Thanks,
>>>
>>> --
>>> Clément Béra
>>> https://clementbera.wordpress.com/
>>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

Eliot Miranda-2
 
Hi Alistair,


> On Dec 22, 2017, at 8:33 AM, Alistair Grant <[hidden email]> wrote:
>
>
> Hi Eliot,
>
>> On 22 December 2017 at 17:09, Eliot Miranda <[hidden email]> wrote:
>>
>> Hi Clément, Hi Alistair,
>>
>>    FileAttrbutesPlugin is not yet ready for use.  The main problem is that it neither fails properly nor communicates error codes back properly.  Basically FileAttrbutesPlugin confuses failing because it gets invalid arguments with reporting error codes due to operating system level errors.  For the FileAttrbutesPlugin's primitives to work in Spur they must fail in the traditional way when given invalid arguments.  This is because the Spur VM will check when a primitive fails if there are any forwarders in the primitive's parameters, and if so, follow (fix up) the forwarders and retry the primitive.
>
> Is this the feedback you gave me on 2 September, or something different?

One and the same.

>
> Back then, you wrote:
>> The change that is required to error handling:
>> If a primitive fails due to argument marshaling it must use the primitiveFailedWith: mechanism and report the error using one of the error codes.  This is because Spur uses lazy forwarding to implement become:, pin, et al.  The lazy forwarding mechanism follows forwarders in the VM when they are encountered.  For sends this is trivial; any send to a forwarder fails and so the failure side of message lookup follows forwarders and retries the send.  Similarly for the arguments of primitives, when a primitive fails the VM searches for forwarders in the receiver and arguments to a specific pre-computed depth, following any forwarders it finds.  If any forwarders are found the primitive is retried after following all the forwarders found.
>
>
> My understanding is that this has already been done.  If I've
> misunderstood, my apologies.

I should apologize.  One side effect of answering email in my phone is that I don't look things up as often as I should (blush).  But answering email in bed in the early morning while the kids are still asleep is irresistible.  Anyway, good.  I shall try and review the code this weekend or early next week.  Feel free to nag me.  Feel free to use harsh language ;-)


>
>
>> Next, communication no os errors back through the primitives results is acceptable, but a better way would be to use the new #'operating system error' primitive failure code mechanism.  Here, provided a suitable two slot prototype is installed in the primitiveErrorTable (see Squeak trunk) then a plugin can do
>>        interpreterProxy primitiveFailForOSError(signedSixtyFourBitErrorCode)
>> and the VM will clone the error object whose first slot should be #'operating system error' and whose second slot will be the code.
>>
>> This gives us a clean way of communicating errors back to a primitive's method body in the traditional way.
>
> At the moment I'm passing back an error number generated by the
> plugin, not an OS error number returned by a system call, but I assume
> that the VM doesn't really care.
>
> Do you have an example where this is being used in a plugin?

Not yet.  This is new.  Monty wanted if god the FilePlugin and we really needed it.  I'll prepare a proper email with examples of usage and a class def for the error prototype soon.  Again, harsh language may be effective ;-)

> Cheers,
> Alistair

Cheers!

>
>
>> I do apologize if I hadn't made this clear earlier.  But AFAIA there is significant work to do before the plugin is ready for integration.
>>
>> _,,,^..^,,,_ (phone)
>>
>>> On Dec 22, 2017, at 2:48 AM, Alistair Grant <[hidden email]> wrote:
>>>
>>>
>>> Hi Clément,
>>>
>>>> On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> What is the status of the FilesAttributesPlugin ?
>>>
>>> I'm hoping that it will be integrated soon.
>>>
>>>
>>>> I would like to make it available by default on pharo VMs.
>>>
>>> Yes, please. :-)
>>>
>>>
>>>> Questions:
>>>>
>>>> 1) Is it already merged with another plugin ?
>>>
>>> I'm not sure that I understand the question.  FileAttributesPlugin
>>> works along side FilePlugin (which is obviously already part of the
>>> pharo VM).
>>>
>>>
>>>> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
>>>
>>> That's my guess, but someone knowledgable needs to answer this.
>>>
>>>
>>>> 3) What is the right way to generate a plugin ? I use this:
>>>>
>>>> (VMMaker
>>>> makerFor: StackInterpreter
>>>> and: nil
>>>> with: #()
>>>> to: (FileDirectory default pathFromURI: '../src')
>>>> platformDir: (FileDirectory default pathFromURI: '../platforms')
>>>> including: #(FileAttributesPlugin)
>>>> ) generateExternalPlugin: FileAttributesPlugin
>>>>
>>>> Is that correct ?
>>>> It seems it generates only the C file but no header file.
>>>
>>> There isn't a FileAttributesPlugin.h.  It does need the platform support file:
>>>
>>> platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin
>>>
>>> which is already part of opensmalltalk-vm.
>>>
>>> I've always used VMMakerTool to make the plugin, but if it's
>>> generating the .c file it's probably correct.
>>>
>>>
>>> My plan was that once the plugin was added to VMMaker-Plugins that I
>>> would test it again on linux and windows, and then ask for the
>>> appropriate plugins.int to be updated.
>>>
>>>
>>> Thanks!
>>> Alistair
>>>
>>>
>>>> Thanks,
>>>>
>>>> --
>>>> Clément Béra
>>>> https://clementbera.wordpress.com/
>>>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

alistairgrant
 
Hi Eliot,

On 22 December 2017 at 18:00, Eliot Miranda <[hidden email]> wrote:

>
> Hi Alistair,
>
>
>> On Dec 22, 2017, at 8:33 AM, Alistair Grant <[hidden email]> wrote:
>>
>>
>> Hi Eliot,
>>
>>> On 22 December 2017 at 17:09, Eliot Miranda <[hidden email]> wrote:
>>>
>>> Hi Clément, Hi Alistair,
>>>
>>>    FileAttrbutesPlugin is not yet ready for use.  The main problem is that it neither fails properly nor communicates error codes back properly.  Basically FileAttrbutesPlugin confuses failing because it gets invalid arguments with reporting error codes due to operating system level errors.  For the FileAttrbutesPlugin's primitives to work in Spur they must fail in the traditional way when given invalid arguments.  This is because the Spur VM will check when a primitive fails if there are any forwarders in the primitive's parameters, and if so, follow (fix up) the forwarders and retry the primitive.
>>
>> Is this the feedback you gave me on 2 September, or something different?
>
> One and the same.
>
>>
>> Back then, you wrote:
>>> The change that is required to error handling:
>>> If a primitive fails due to argument marshaling it must use the primitiveFailedWith: mechanism and report the error using one of the error codes.  This is because Spur uses lazy forwarding to implement become:, pin, et al.  The lazy forwarding mechanism follows forwarders in the VM when they are encountered.  For sends this is trivial; any send to a forwarder fails and so the failure side of message lookup follows forwarders and retries the send.  Similarly for the arguments of primitives, when a primitive fails the VM searches for forwarders in the receiver and arguments to a specific pre-computed depth, following any forwarders it finds.  If any forwarders are found the primitive is retried after following all the forwarders found.
>>
>>
>> My understanding is that this has already been done.  If I've
>> misunderstood, my apologies.
>
> I should apologize.  One side effect of answering email in my phone is that I don't look things up as often as I should (blush).  But answering email in bed in the early morning while the kids are still asleep is irresistible.  Anyway, good.  I shall try and review the code this weekend or early next week.  Feel free to nag me.  Feel free to use harsh language ;-)

No problem.    The email subject where I run over the mods is "Re:
20294 Add FileAttributesPlugin to the linux 32 bit VM", sent 6 Sep
2017.



>>> Next, communication no os errors back through the primitives results is acceptable, but a better way would be to use the new #'operating system error' primitive failure code mechanism.  Here, provided a suitable two slot prototype is installed in the primitiveErrorTable (see Squeak trunk) then a plugin can do
>>>        interpreterProxy primitiveFailForOSError(signedSixtyFourBitErrorCode)
>>> and the VM will clone the error object whose first slot should be #'operating system error' and whose second slot will be the code.
>>>
>>> This gives us a clean way of communicating errors back to a primitive's method body in the traditional way.
>>
>> At the moment I'm passing back an error number generated by the
>> plugin, not an OS error number returned by a system call, but I assume
>> that the VM doesn't really care.
>>
>> Do you have an example where this is being used in a plugin?
>
> Not yet.  This is new.  Monty wanted if god the FilePlugin and we really needed it.  I'll prepare a proper email with examples of usage and a class def for the error prototype soon.  Again, harsh language may be effective ;-)

OK.  I'll have a look, but the examples will be good.

Thanks,
Alistair



>> Cheers,
>> Alistair
>
> Cheers!
>
>>
>>
>>> I do apologize if I hadn't made this clear earlier.  But AFAIA there is significant work to do before the plugin is ready for integration.
>>>
>>> _,,,^..^,,,_ (phone)
>>>
>>>> On Dec 22, 2017, at 2:48 AM, Alistair Grant <[hidden email]> wrote:
>>>>
>>>>
>>>> Hi Clément,
>>>>
>>>>> On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> What is the status of the FilesAttributesPlugin ?
>>>>
>>>> I'm hoping that it will be integrated soon.
>>>>
>>>>
>>>>> I would like to make it available by default on pharo VMs.
>>>>
>>>> Yes, please. :-)
>>>>
>>>>
>>>>> Questions:
>>>>>
>>>>> 1) Is it already merged with another plugin ?
>>>>
>>>> I'm not sure that I understand the question.  FileAttributesPlugin
>>>> works along side FilePlugin (which is obviously already part of the
>>>> pharo VM).
>>>>
>>>>
>>>>> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
>>>>
>>>> That's my guess, but someone knowledgable needs to answer this.
>>>>
>>>>
>>>>> 3) What is the right way to generate a plugin ? I use this:
>>>>>
>>>>> (VMMaker
>>>>> makerFor: StackInterpreter
>>>>> and: nil
>>>>> with: #()
>>>>> to: (FileDirectory default pathFromURI: '../src')
>>>>> platformDir: (FileDirectory default pathFromURI: '../platforms')
>>>>> including: #(FileAttributesPlugin)
>>>>> ) generateExternalPlugin: FileAttributesPlugin
>>>>>
>>>>> Is that correct ?
>>>>> It seems it generates only the C file but no header file.
>>>>
>>>> There isn't a FileAttributesPlugin.h.  It does need the platform support file:
>>>>
>>>> platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin
>>>>
>>>> which is already part of opensmalltalk-vm.
>>>>
>>>> I've always used VMMakerTool to make the plugin, but if it's
>>>> generating the .c file it's probably correct.
>>>>
>>>>
>>>> My plan was that once the plugin was added to VMMaker-Plugins that I
>>>> would test it again on linux and windows, and then ask for the
>>>> appropriate plugins.int to be updated.
>>>>
>>>>
>>>> Thanks!
>>>> Alistair
>>>>
>>>>
>>>>> Thanks,
>>>>>
>>>>> --
>>>>> Clément Béra
>>>>> https://clementbera.wordpress.com/
>>>>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

alistairgrant
 
Hi Eliot,

On 22 December 2017 at 18:11, Alistair Grant <[hidden email]> wrote:

> Hi Eliot,
>
> On 22 December 2017 at 18:00, Eliot Miranda <[hidden email]> wrote:
>>
>> Hi Alistair,
>>
>>
>>> On Dec 22, 2017, at 8:33 AM, Alistair Grant <[hidden email]> wrote:
>>>
>>>
>>> Hi Eliot,
>>>
>>>> On 22 December 2017 at 17:09, Eliot Miranda <[hidden email]> wrote:
>>>>
>>>> Hi Clément, Hi Alistair,
>>>>
>>>>    FileAttrbutesPlugin is not yet ready for use.  The main problem is that it neither fails properly nor communicates error codes back properly.  Basically FileAttrbutesPlugin confuses failing because it gets invalid arguments with reporting error codes due to operating system level errors.  For the FileAttrbutesPlugin's primitives to work in Spur they must fail in the traditional way when given invalid arguments.  This is because the Spur VM will check when a primitive fails if there are any forwarders in the primitive's parameters, and if so, follow (fix up) the forwarders and retry the primitive.
>>>
>>> Is this the feedback you gave me on 2 September, or something different?
>>
>> One and the same.
>>
>>>
>>> Back then, you wrote:
>>>> The change that is required to error handling:
>>>> If a primitive fails due to argument marshaling it must use the primitiveFailedWith: mechanism and report the error using one of the error codes.  This is because Spur uses lazy forwarding to implement become:, pin, et al.  The lazy forwarding mechanism follows forwarders in the VM when they are encountered.  For sends this is trivial; any send to a forwarder fails and so the failure side of message lookup follows forwarders and retries the send.  Similarly for the arguments of primitives, when a primitive fails the VM searches for forwarders in the receiver and arguments to a specific pre-computed depth, following any forwarders it finds.  If any forwarders are found the primitive is retried after following all the forwarders found.
>>>
>>>
>>> My understanding is that this has already been done.  If I've
>>> misunderstood, my apologies.
>>
>> I should apologize.  One side effect of answering email in my phone is that I don't look things up as often as I should (blush).  But answering email in bed in the early morning while the kids are still asleep is irresistible.  Anyway, good.  I shall try and review the code this weekend or early next week.  Feel free to nag me.  Feel free to use harsh language ;-)

And of course I've found one spot where I failed to fix the argument
validation (in #primitiveRewinddir).  I'll modify the method to call
#primitiveFailFor:.

Cheers,
Alistair



> No problem.    The email subject where I run over the mods is "Re:
> 20294 Add FileAttributesPlugin to the linux 32 bit VM", sent 6 Sep
> 2017.
>
>
>
>>>> Next, communication no os errors back through the primitives results is acceptable, but a better way would be to use the new #'operating system error' primitive failure code mechanism.  Here, provided a suitable two slot prototype is installed in the primitiveErrorTable (see Squeak trunk) then a plugin can do
>>>>        interpreterProxy primitiveFailForOSError(signedSixtyFourBitErrorCode)
>>>> and the VM will clone the error object whose first slot should be #'operating system error' and whose second slot will be the code.
>>>>
>>>> This gives us a clean way of communicating errors back to a primitive's method body in the traditional way.
>>>
>>> At the moment I'm passing back an error number generated by the
>>> plugin, not an OS error number returned by a system call, but I assume
>>> that the VM doesn't really care.
>>>
>>> Do you have an example where this is being used in a plugin?
>>
>> Not yet.  This is new.  Monty wanted if god the FilePlugin and we really needed it.  I'll prepare a proper email with examples of usage and a class def for the error prototype soon.  Again, harsh language may be effective ;-)
>
> OK.  I'll have a look, but the examples will be good.
>
> Thanks,
> Alistair
>
>
>
>>> Cheers,
>>> Alistair
>>
>> Cheers!
>>
>>>
>>>
>>>> I do apologize if I hadn't made this clear earlier.  But AFAIA there is significant work to do before the plugin is ready for integration.
>>>>
>>>> _,,,^..^,,,_ (phone)
>>>>
>>>>> On Dec 22, 2017, at 2:48 AM, Alistair Grant <[hidden email]> wrote:
>>>>>
>>>>>
>>>>> Hi Clément,
>>>>>
>>>>>> On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> What is the status of the FilesAttributesPlugin ?
>>>>>
>>>>> I'm hoping that it will be integrated soon.
>>>>>
>>>>>
>>>>>> I would like to make it available by default on pharo VMs.
>>>>>
>>>>> Yes, please. :-)
>>>>>
>>>>>
>>>>>> Questions:
>>>>>>
>>>>>> 1) Is it already merged with another plugin ?
>>>>>
>>>>> I'm not sure that I understand the question.  FileAttributesPlugin
>>>>> works along side FilePlugin (which is obviously already part of the
>>>>> pharo VM).
>>>>>
>>>>>
>>>>>> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
>>>>>
>>>>> That's my guess, but someone knowledgable needs to answer this.
>>>>>
>>>>>
>>>>>> 3) What is the right way to generate a plugin ? I use this:
>>>>>>
>>>>>> (VMMaker
>>>>>> makerFor: StackInterpreter
>>>>>> and: nil
>>>>>> with: #()
>>>>>> to: (FileDirectory default pathFromURI: '../src')
>>>>>> platformDir: (FileDirectory default pathFromURI: '../platforms')
>>>>>> including: #(FileAttributesPlugin)
>>>>>> ) generateExternalPlugin: FileAttributesPlugin
>>>>>>
>>>>>> Is that correct ?
>>>>>> It seems it generates only the C file but no header file.
>>>>>
>>>>> There isn't a FileAttributesPlugin.h.  It does need the platform support file:
>>>>>
>>>>> platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin
>>>>>
>>>>> which is already part of opensmalltalk-vm.
>>>>>
>>>>> I've always used VMMakerTool to make the plugin, but if it's
>>>>> generating the .c file it's probably correct.
>>>>>
>>>>>
>>>>> My plan was that once the plugin was added to VMMaker-Plugins that I
>>>>> would test it again on linux and windows, and then ask for the
>>>>> appropriate plugins.int to be updated.
>>>>>
>>>>>
>>>>> Thanks!
>>>>> Alistair
>>>>>
>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> --
>>>>>> Clément Béra
>>>>>> https://clementbera.wordpress.com/
>>>>>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

Eliot Miranda-2
In reply to this post by alistairgrant
 
Hi Alistair,  Hi Clément,

  here are the code and an example of the new operating system error primitive fail facility.

Here's the extract from recreateSpecialObjectsArray that adds the prototype to the table:

newArray at: 52 put: #(nil "nil => generic error" #'bad receiver'
#'bad argument' #'bad index'
#'bad number of arguments'
#'inappropriate operation'  #'unsupported operation'
#'no modification' #'insufficient object memory'
#'insufficient C memory' #'not found' #'bad method'
#'internal error in named primitive machinery'
#'object may move' #'resource limit exceeded'
#'object is pinned' #'primitive write beyond end of object'
#'object moved' #'object not pinned' #'callback error'),
{PrimitiveOSError new errorName: #'operating system error'; yourself}.

Here's the class definition (also attached) for PrimitiveOSError:

!Object methodsFor: '*System-Support-error handling' stamp: 'eem 12/12/2017 09:16'!
isPrimitiveOSError
^false! !

Object subclass: #PrimitiveOSError
instanceVariableNames: 'errorName errorCode'
classVariableNames: ''
poolDictionaries: ''
category: 'System-Support'!
!PrimitiveOSError commentStamp: 'eem 12/7/2017 19:31' prior: 0!
A PrimitiveOSError is used to answer a primitive failure code that has an associated operating system/library error.

Instance Variables
errorName: <Symbol>
errorValue: <Integer>

errorName
- typically #'operating system error'

errorValue
- the value of the error, a signed 64-bit value, a representation imposed by the VM; specific clients must map this error value into an unsigned value as appropriate if required!


!PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 19:32'!
errorCode
^errorCode! !

!PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 19:32'!
errorCode: anObject
errorCode := anObject! !

!PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 18:16'!
errorName
^errorName! !

!PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 18:16'!
errorName: anObject
errorName := anObject! !


!PrimitiveOSError methodsFor: 'testing' stamp: 'eem 12/12/2017 09:15'!
isPrimitiveOSError
^true! !


The signature of primitiveFailForOSError: is:
sqInt primitiveFailForOSError(sqLong)

So in your Slang code use

    interpreterProxy primitiveFailForOSError: osErrorCode

or in your platform C code use


    interpreterProxy->primitiveFailForOSError(osErrorCode)

In your Smalltalk code write things like

     <primitive: 'foo' module: '' error: ec>
     ec isOSErrorCode ifTrue:
         [self processError: ec errorCode]




On Fri, Dec 22, 2017 at 9:11 AM, Alistair Grant <[hidden email]> wrote:

Hi Eliot,

On 22 December 2017 at 18:00, Eliot Miranda <[hidden email]> wrote:
>
> Hi Alistair,
>
>
>> On Dec 22, 2017, at 8:33 AM, Alistair Grant <[hidden email]> wrote:
>>
>>
>> Hi Eliot,
>>
>>> On 22 December 2017 at 17:09, Eliot Miranda <[hidden email]> wrote:
>>>
>>> Hi Clément, Hi Alistair,
>>>
>>>    FileAttrbutesPlugin is not yet ready for use.  The main problem is that it neither fails properly nor communicates error codes back properly.  Basically FileAttrbutesPlugin confuses failing because it gets invalid arguments with reporting error codes due to operating system level errors.  For the FileAttrbutesPlugin's primitives to work in Spur they must fail in the traditional way when given invalid arguments.  This is because the Spur VM will check when a primitive fails if there are any forwarders in the primitive's parameters, and if so, follow (fix up) the forwarders and retry the primitive.
>>
>> Is this the feedback you gave me on 2 September, or something different?
>
> One and the same.
>
>>
>> Back then, you wrote:
>>> The change that is required to error handling:
>>> If a primitive fails due to argument marshaling it must use the primitiveFailedWith: mechanism and report the error using one of the error codes.  This is because Spur uses lazy forwarding to implement become:, pin, et al.  The lazy forwarding mechanism follows forwarders in the VM when they are encountered.  For sends this is trivial; any send to a forwarder fails and so the failure side of message lookup follows forwarders and retries the send.  Similarly for the arguments of primitives, when a primitive fails the VM searches for forwarders in the receiver and arguments to a specific pre-computed depth, following any forwarders it finds.  If any forwarders are found the primitive is retried after following all the forwarders found.
>>
>>
>> My understanding is that this has already been done.  If I've
>> misunderstood, my apologies.
>
> I should apologize.  One side effect of answering email in my phone is that I don't look things up as often as I should (blush).  But answering email in bed in the early morning while the kids are still asleep is irresistible.  Anyway, good.  I shall try and review the code this weekend or early next week.  Feel free to nag me.  Feel free to use harsh language ;-)

No problem.    The email subject where I run over the mods is "Re:
20294 Add FileAttributesPlugin to the linux 32 bit VM", sent 6 Sep
2017.



>>> Next, communication no os errors back through the primitives results is acceptable, but a better way would be to use the new #'operating system error' primitive failure code mechanism.  Here, provided a suitable two slot prototype is installed in the primitiveErrorTable (see Squeak trunk) then a plugin can do
>>>        interpreterProxy primitiveFailForOSError(signedSixtyFourBitErrorCode)
>>> and the VM will clone the error object whose first slot should be #'operating system error' and whose second slot will be the code.
>>>
>>> This gives us a clean way of communicating errors back to a primitive's method body in the traditional way.
>>
>> At the moment I'm passing back an error number generated by the
>> plugin, not an OS error number returned by a system call, but I assume
>> that the VM doesn't really care.
>>
>> Do you have an example where this is being used in a plugin?
>
> Not yet.  This is new.  Monty wanted if god the FilePlugin and we really needed it.  I'll prepare a proper email with examples of usage and a class def for the error prototype soon.  Again, harsh language may be effective ;-)

OK.  I'll have a look, but the examples will be good.

Thanks,
Alistair



>> Cheers,
>> Alistair
>
> Cheers!
>
>>
>>
>>> I do apologize if I hadn't made this clear earlier.  But AFAIA there is significant work to do before the plugin is ready for integration.
>>>
>>> _,,,^..^,,,_ (phone)
>>>
>>>> On Dec 22, 2017, at 2:48 AM, Alistair Grant <[hidden email]> wrote:
>>>>
>>>>
>>>> Hi Clément,
>>>>
>>>>> On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> What is the status of the FilesAttributesPlugin ?
>>>>
>>>> I'm hoping that it will be integrated soon.
>>>>
>>>>
>>>>> I would like to make it available by default on pharo VMs.
>>>>
>>>> Yes, please. :-)
>>>>
>>>>
>>>>> Questions:
>>>>>
>>>>> 1) Is it already merged with another plugin ?
>>>>
>>>> I'm not sure that I understand the question.  FileAttributesPlugin
>>>> works along side FilePlugin (which is obviously already part of the
>>>> pharo VM).
>>>>
>>>>
>>>>> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
>>>>
>>>> That's my guess, but someone knowledgable needs to answer this.
>>>>
>>>>
>>>>> 3) What is the right way to generate a plugin ? I use this:
>>>>>
>>>>> (VMMaker
>>>>> makerFor: StackInterpreter
>>>>> and: nil
>>>>> with: #()
>>>>> to: (FileDirectory default pathFromURI: '../src')
>>>>> platformDir: (FileDirectory default pathFromURI: '../platforms')
>>>>> including: #(FileAttributesPlugin)
>>>>> ) generateExternalPlugin: FileAttributesPlugin
>>>>>
>>>>> Is that correct ?
>>>>> It seems it generates only the C file but no header file.
>>>>
>>>> There isn't a FileAttributesPlugin.h.  It does need the platform support file:
>>>>
>>>> platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin
>>>>
>>>> which is already part of opensmalltalk-vm.
>>>>
>>>> I've always used VMMakerTool to make the plugin, but if it's
>>>> generating the .c file it's probably correct.
>>>>
>>>>
>>>> My plan was that once the plugin was added to VMMaker-Plugins that I
>>>> would test it again on linux and windows, and then ask for the
>>>> appropriate plugins.int to be updated.
>>>>
>>>>
>>>> Thanks!
>>>> Alistair
>>>>
>>>>
>>>>> Thanks,
>>>>>
>>>>> --
>>>>> Clément Béra
>>>>> https://clementbera.wordpress.com/
>>>>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq



--
_,,,^..^,,,_
best, Eliot
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

alistairgrant
 
Hi Eliot,

Thanks!  I'd figured out the slang changes, I'll start work on the
Smalltalk code changes after I've fixed my vm build system.

Cheers,
Alistair


On 22 December 2017 at 20:59, Eliot Miranda <[hidden email]> wrote:

>
> Hi Alistair,  Hi Clément,
>
>   here are the code and an example of the new operating system error primitive fail facility.
>
> Here's the extract from recreateSpecialObjectsArray that adds the prototype to the table:
>
> newArray at: 52 put: #(nil "nil => generic error" #'bad receiver'
> #'bad argument' #'bad index'
> #'bad number of arguments'
> #'inappropriate operation'  #'unsupported operation'
> #'no modification' #'insufficient object memory'
> #'insufficient C memory' #'not found' #'bad method'
> #'internal error in named primitive machinery'
> #'object may move' #'resource limit exceeded'
> #'object is pinned' #'primitive write beyond end of object'
> #'object moved' #'object not pinned' #'callback error'),
> {PrimitiveOSError new errorName: #'operating system error'; yourself}.
>
> Here's the class definition (also attached) for PrimitiveOSError:
>
> !Object methodsFor: '*System-Support-error handling' stamp: 'eem 12/12/2017 09:16'!
> isPrimitiveOSError
> ^false! !
>
> Object subclass: #PrimitiveOSError
> instanceVariableNames: 'errorName errorCode'
> classVariableNames: ''
> poolDictionaries: ''
> category: 'System-Support'!
> !PrimitiveOSError commentStamp: 'eem 12/7/2017 19:31' prior: 0!
> A PrimitiveOSError is used to answer a primitive failure code that has an associated operating system/library error.
>
> Instance Variables
> errorName: <Symbol>
> errorValue: <Integer>
>
> errorName
> - typically #'operating system error'
>
> errorValue
> - the value of the error, a signed 64-bit value, a representation imposed by the VM; specific clients must map this error value into an unsigned value as appropriate if required!
>
>
> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 19:32'!
> errorCode
> ^errorCode! !
>
> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 19:32'!
> errorCode: anObject
> errorCode := anObject! !
>
> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 18:16'!
> errorName
> ^errorName! !
>
> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 18:16'!
> errorName: anObject
> errorName := anObject! !
>
>
> !PrimitiveOSError methodsFor: 'testing' stamp: 'eem 12/12/2017 09:15'!
> isPrimitiveOSError
> ^true! !
>
>
> The signature of primitiveFailForOSError: is:
> sqInt primitiveFailForOSError(sqLong)
>
> So in your Slang code use
>
>     interpreterProxy primitiveFailForOSError: osErrorCode
>
> or in your platform C code use
>
>
>     interpreterProxy->primitiveFailForOSError(osErrorCode)
>
> In your Smalltalk code write things like
>
>      <primitive: 'foo' module: '' error: ec>
>      ec isOSErrorCode ifTrue:
>          [self processError: ec errorCode]
>
>
>
>
> On Fri, Dec 22, 2017 at 9:11 AM, Alistair Grant <[hidden email]> wrote:
>>
>>
>> Hi Eliot,
>>
>> On 22 December 2017 at 18:00, Eliot Miranda <[hidden email]> wrote:
>> >
>> > Hi Alistair,
>> >
>> >
>> >> On Dec 22, 2017, at 8:33 AM, Alistair Grant <[hidden email]> wrote:
>> >>
>> >>
>> >> Hi Eliot,
>> >>
>> >>> On 22 December 2017 at 17:09, Eliot Miranda <[hidden email]> wrote:
>> >>>
>> >>> Hi Clément, Hi Alistair,
>> >>>
>> >>>    FileAttrbutesPlugin is not yet ready for use.  The main problem is that it neither fails properly nor communicates error codes back properly.  Basically FileAttrbutesPlugin confuses failing because it gets invalid arguments with reporting error codes due to operating system level errors.  For the FileAttrbutesPlugin's primitives to work in Spur they must fail in the traditional way when given invalid arguments.  This is because the Spur VM will check when a primitive fails if there are any forwarders in the primitive's parameters, and if so, follow (fix up) the forwarders and retry the primitive.
>> >>
>> >> Is this the feedback you gave me on 2 September, or something different?
>> >
>> > One and the same.
>> >
>> >>
>> >> Back then, you wrote:
>> >>> The change that is required to error handling:
>> >>> If a primitive fails due to argument marshaling it must use the primitiveFailedWith: mechanism and report the error using one of the error codes.  This is because Spur uses lazy forwarding to implement become:, pin, et al.  The lazy forwarding mechanism follows forwarders in the VM when they are encountered.  For sends this is trivial; any send to a forwarder fails and so the failure side of message lookup follows forwarders and retries the send.  Similarly for the arguments of primitives, when a primitive fails the VM searches for forwarders in the receiver and arguments to a specific pre-computed depth, following any forwarders it finds.  If any forwarders are found the primitive is retried after following all the forwarders found.
>> >>
>> >>
>> >> My understanding is that this has already been done.  If I've
>> >> misunderstood, my apologies.
>> >
>> > I should apologize.  One side effect of answering email in my phone is that I don't look things up as often as I should (blush).  But answering email in bed in the early morning while the kids are still asleep is irresistible.  Anyway, good.  I shall try and review the code this weekend or early next week.  Feel free to nag me.  Feel free to use harsh language ;-)
>>
>> No problem.    The email subject where I run over the mods is "Re:
>> 20294 Add FileAttributesPlugin to the linux 32 bit VM", sent 6 Sep
>> 2017.
>>
>>
>>
>> >>> Next, communication no os errors back through the primitives results is acceptable, but a better way would be to use the new #'operating system error' primitive failure code mechanism.  Here, provided a suitable two slot prototype is installed in the primitiveErrorTable (see Squeak trunk) then a plugin can do
>> >>>        interpreterProxy primitiveFailForOSError(signedSixtyFourBitErrorCode)
>> >>> and the VM will clone the error object whose first slot should be #'operating system error' and whose second slot will be the code.
>> >>>
>> >>> This gives us a clean way of communicating errors back to a primitive's method body in the traditional way.
>> >>
>> >> At the moment I'm passing back an error number generated by the
>> >> plugin, not an OS error number returned by a system call, but I assume
>> >> that the VM doesn't really care.
>> >>
>> >> Do you have an example where this is being used in a plugin?
>> >
>> > Not yet.  This is new.  Monty wanted if god the FilePlugin and we really needed it.  I'll prepare a proper email with examples of usage and a class def for the error prototype soon.  Again, harsh language may be effective ;-)
>>
>> OK.  I'll have a look, but the examples will be good.
>>
>> Thanks,
>> Alistair
>>
>>
>>
>> >> Cheers,
>> >> Alistair
>> >
>> > Cheers!
>> >
>> >>
>> >>
>> >>> I do apologize if I hadn't made this clear earlier.  But AFAIA there is significant work to do before the plugin is ready for integration.
>> >>>
>> >>> _,,,^..^,,,_ (phone)
>> >>>
>> >>>> On Dec 22, 2017, at 2:48 AM, Alistair Grant <[hidden email]> wrote:
>> >>>>
>> >>>>
>> >>>> Hi Clément,
>> >>>>
>> >>>>> On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>> >>>>>
>> >>>>> Hi,
>> >>>>>
>> >>>>> What is the status of the FilesAttributesPlugin ?
>> >>>>
>> >>>> I'm hoping that it will be integrated soon.
>> >>>>
>> >>>>
>> >>>>> I would like to make it available by default on pharo VMs.
>> >>>>
>> >>>> Yes, please. :-)
>> >>>>
>> >>>>
>> >>>>> Questions:
>> >>>>>
>> >>>>> 1) Is it already merged with another plugin ?
>> >>>>
>> >>>> I'm not sure that I understand the question.  FileAttributesPlugin
>> >>>> works along side FilePlugin (which is obviously already part of the
>> >>>> pharo VM).
>> >>>>
>> >>>>
>> >>>>> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
>> >>>>
>> >>>> That's my guess, but someone knowledgable needs to answer this.
>> >>>>
>> >>>>
>> >>>>> 3) What is the right way to generate a plugin ? I use this:
>> >>>>>
>> >>>>> (VMMaker
>> >>>>> makerFor: StackInterpreter
>> >>>>> and: nil
>> >>>>> with: #()
>> >>>>> to: (FileDirectory default pathFromURI: '../src')
>> >>>>> platformDir: (FileDirectory default pathFromURI: '../platforms')
>> >>>>> including: #(FileAttributesPlugin)
>> >>>>> ) generateExternalPlugin: FileAttributesPlugin
>> >>>>>
>> >>>>> Is that correct ?
>> >>>>> It seems it generates only the C file but no header file.
>> >>>>
>> >>>> There isn't a FileAttributesPlugin.h.  It does need the platform support file:
>> >>>>
>> >>>> platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin
>> >>>>
>> >>>> which is already part of opensmalltalk-vm.
>> >>>>
>> >>>> I've always used VMMakerTool to make the plugin, but if it's
>> >>>> generating the .c file it's probably correct.
>> >>>>
>> >>>>
>> >>>> My plan was that once the plugin was added to VMMaker-Plugins that I
>> >>>> would test it again on linux and windows, and then ask for the
>> >>>> appropriate plugins.int to be updated.
>> >>>>
>> >>>>
>> >>>> Thanks!
>> >>>> Alistair
>> >>>>
>> >>>>
>> >>>>> Thanks,
>> >>>>>
>> >>>>> --
>> >>>>> Clément Béra
>> >>>>> https://clementbera.wordpress.com/
>> >>>>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
>
>
>
>
> --
> _,,,^..^,,,_
> best, Eliot
>
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

alistairgrant
 
Hi Eliot,

I've made progress:

- Updated FileAttributesPlugin to use #primitiveFailForOSError:.
- Loaded PrimitiveOSError
- Updated SmalltalkImage>>newSpecialObjectsArray
- Run SmalltalkImage current recreateSpecialObjectsArray.
- And triggered an error which generated the appropriate
PrimitiveOSError object.

Given I've already made changes, and have a bit more tidying up to do,
it is probably isn't worthwhile you reviewing the code again until I
post an updated version.  I'll let you know when it is ready.

Merry Christmas!
Alistair




On 22 December 2017 at 21:08, Alistair Grant <[hidden email]> wrote:

> Hi Eliot,
>
> Thanks!  I'd figured out the slang changes, I'll start work on the
> Smalltalk code changes after I've fixed my vm build system.
>
> Cheers,
> Alistair
>
>
> On 22 December 2017 at 20:59, Eliot Miranda <[hidden email]> wrote:
>>
>> Hi Alistair,  Hi Clément,
>>
>>   here are the code and an example of the new operating system error primitive fail facility.
>>
>> Here's the extract from recreateSpecialObjectsArray that adds the prototype to the table:
>>
>> newArray at: 52 put: #(nil "nil => generic error" #'bad receiver'
>> #'bad argument' #'bad index'
>> #'bad number of arguments'
>> #'inappropriate operation'  #'unsupported operation'
>> #'no modification' #'insufficient object memory'
>> #'insufficient C memory' #'not found' #'bad method'
>> #'internal error in named primitive machinery'
>> #'object may move' #'resource limit exceeded'
>> #'object is pinned' #'primitive write beyond end of object'
>> #'object moved' #'object not pinned' #'callback error'),
>> {PrimitiveOSError new errorName: #'operating system error'; yourself}.
>>
>> Here's the class definition (also attached) for PrimitiveOSError:
>>
>> !Object methodsFor: '*System-Support-error handling' stamp: 'eem 12/12/2017 09:16'!
>> isPrimitiveOSError
>> ^false! !
>>
>> Object subclass: #PrimitiveOSError
>> instanceVariableNames: 'errorName errorCode'
>> classVariableNames: ''
>> poolDictionaries: ''
>> category: 'System-Support'!
>> !PrimitiveOSError commentStamp: 'eem 12/7/2017 19:31' prior: 0!
>> A PrimitiveOSError is used to answer a primitive failure code that has an associated operating system/library error.
>>
>> Instance Variables
>> errorName: <Symbol>
>> errorValue: <Integer>
>>
>> errorName
>> - typically #'operating system error'
>>
>> errorValue
>> - the value of the error, a signed 64-bit value, a representation imposed by the VM; specific clients must map this error value into an unsigned value as appropriate if required!
>>
>>
>> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 19:32'!
>> errorCode
>> ^errorCode! !
>>
>> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 19:32'!
>> errorCode: anObject
>> errorCode := anObject! !
>>
>> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 18:16'!
>> errorName
>> ^errorName! !
>>
>> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 18:16'!
>> errorName: anObject
>> errorName := anObject! !
>>
>>
>> !PrimitiveOSError methodsFor: 'testing' stamp: 'eem 12/12/2017 09:15'!
>> isPrimitiveOSError
>> ^true! !
>>
>>
>> The signature of primitiveFailForOSError: is:
>> sqInt primitiveFailForOSError(sqLong)
>>
>> So in your Slang code use
>>
>>     interpreterProxy primitiveFailForOSError: osErrorCode
>>
>> or in your platform C code use
>>
>>
>>     interpreterProxy->primitiveFailForOSError(osErrorCode)
>>
>> In your Smalltalk code write things like
>>
>>      <primitive: 'foo' module: '' error: ec>
>>      ec isOSErrorCode ifTrue:
>>          [self processError: ec errorCode]
>>
>>
>>
>>
>> On Fri, Dec 22, 2017 at 9:11 AM, Alistair Grant <[hidden email]> wrote:
>>>
>>>
>>> Hi Eliot,
>>>
>>> On 22 December 2017 at 18:00, Eliot Miranda <[hidden email]> wrote:
>>> >
>>> > Hi Alistair,
>>> >
>>> >
>>> >> On Dec 22, 2017, at 8:33 AM, Alistair Grant <[hidden email]> wrote:
>>> >>
>>> >>
>>> >> Hi Eliot,
>>> >>
>>> >>> On 22 December 2017 at 17:09, Eliot Miranda <[hidden email]> wrote:
>>> >>>
>>> >>> Hi Clément, Hi Alistair,
>>> >>>
>>> >>>    FileAttrbutesPlugin is not yet ready for use.  The main problem is that it neither fails properly nor communicates error codes back properly.  Basically FileAttrbutesPlugin confuses failing because it gets invalid arguments with reporting error codes due to operating system level errors.  For the FileAttrbutesPlugin's primitives to work in Spur they must fail in the traditional way when given invalid arguments.  This is because the Spur VM will check when a primitive fails if there are any forwarders in the primitive's parameters, and if so, follow (fix up) the forwarders and retry the primitive.
>>> >>
>>> >> Is this the feedback you gave me on 2 September, or something different?
>>> >
>>> > One and the same.
>>> >
>>> >>
>>> >> Back then, you wrote:
>>> >>> The change that is required to error handling:
>>> >>> If a primitive fails due to argument marshaling it must use the primitiveFailedWith: mechanism and report the error using one of the error codes.  This is because Spur uses lazy forwarding to implement become:, pin, et al.  The lazy forwarding mechanism follows forwarders in the VM when they are encountered.  For sends this is trivial; any send to a forwarder fails and so the failure side of message lookup follows forwarders and retries the send.  Similarly for the arguments of primitives, when a primitive fails the VM searches for forwarders in the receiver and arguments to a specific pre-computed depth, following any forwarders it finds.  If any forwarders are found the primitive is retried after following all the forwarders found.
>>> >>
>>> >>
>>> >> My understanding is that this has already been done.  If I've
>>> >> misunderstood, my apologies.
>>> >
>>> > I should apologize.  One side effect of answering email in my phone is that I don't look things up as often as I should (blush).  But answering email in bed in the early morning while the kids are still asleep is irresistible.  Anyway, good.  I shall try and review the code this weekend or early next week.  Feel free to nag me.  Feel free to use harsh language ;-)
>>>
>>> No problem.    The email subject where I run over the mods is "Re:
>>> 20294 Add FileAttributesPlugin to the linux 32 bit VM", sent 6 Sep
>>> 2017.
>>>
>>>
>>>
>>> >>> Next, communication no os errors back through the primitives results is acceptable, but a better way would be to use the new #'operating system error' primitive failure code mechanism.  Here, provided a suitable two slot prototype is installed in the primitiveErrorTable (see Squeak trunk) then a plugin can do
>>> >>>        interpreterProxy primitiveFailForOSError(signedSixtyFourBitErrorCode)
>>> >>> and the VM will clone the error object whose first slot should be #'operating system error' and whose second slot will be the code.
>>> >>>
>>> >>> This gives us a clean way of communicating errors back to a primitive's method body in the traditional way.
>>> >>
>>> >> At the moment I'm passing back an error number generated by the
>>> >> plugin, not an OS error number returned by a system call, but I assume
>>> >> that the VM doesn't really care.
>>> >>
>>> >> Do you have an example where this is being used in a plugin?
>>> >
>>> > Not yet.  This is new.  Monty wanted if god the FilePlugin and we really needed it.  I'll prepare a proper email with examples of usage and a class def for the error prototype soon.  Again, harsh language may be effective ;-)
>>>
>>> OK.  I'll have a look, but the examples will be good.
>>>
>>> Thanks,
>>> Alistair
>>>
>>>
>>>
>>> >> Cheers,
>>> >> Alistair
>>> >
>>> > Cheers!
>>> >
>>> >>
>>> >>
>>> >>> I do apologize if I hadn't made this clear earlier.  But AFAIA there is significant work to do before the plugin is ready for integration.
>>> >>>
>>> >>> _,,,^..^,,,_ (phone)
>>> >>>
>>> >>>> On Dec 22, 2017, at 2:48 AM, Alistair Grant <[hidden email]> wrote:
>>> >>>>
>>> >>>>
>>> >>>> Hi Clément,
>>> >>>>
>>> >>>>> On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>>> >>>>>
>>> >>>>> Hi,
>>> >>>>>
>>> >>>>> What is the status of the FilesAttributesPlugin ?
>>> >>>>
>>> >>>> I'm hoping that it will be integrated soon.
>>> >>>>
>>> >>>>
>>> >>>>> I would like to make it available by default on pharo VMs.
>>> >>>>
>>> >>>> Yes, please. :-)
>>> >>>>
>>> >>>>
>>> >>>>> Questions:
>>> >>>>>
>>> >>>>> 1) Is it already merged with another plugin ?
>>> >>>>
>>> >>>> I'm not sure that I understand the question.  FileAttributesPlugin
>>> >>>> works along side FilePlugin (which is obviously already part of the
>>> >>>> pharo VM).
>>> >>>>
>>> >>>>
>>> >>>>> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
>>> >>>>
>>> >>>> That's my guess, but someone knowledgable needs to answer this.
>>> >>>>
>>> >>>>
>>> >>>>> 3) What is the right way to generate a plugin ? I use this:
>>> >>>>>
>>> >>>>> (VMMaker
>>> >>>>> makerFor: StackInterpreter
>>> >>>>> and: nil
>>> >>>>> with: #()
>>> >>>>> to: (FileDirectory default pathFromURI: '../src')
>>> >>>>> platformDir: (FileDirectory default pathFromURI: '../platforms')
>>> >>>>> including: #(FileAttributesPlugin)
>>> >>>>> ) generateExternalPlugin: FileAttributesPlugin
>>> >>>>>
>>> >>>>> Is that correct ?
>>> >>>>> It seems it generates only the C file but no header file.
>>> >>>>
>>> >>>> There isn't a FileAttributesPlugin.h.  It does need the platform support file:
>>> >>>>
>>> >>>> platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin
>>> >>>>
>>> >>>> which is already part of opensmalltalk-vm.
>>> >>>>
>>> >>>> I've always used VMMakerTool to make the plugin, but if it's
>>> >>>> generating the .c file it's probably correct.
>>> >>>>
>>> >>>>
>>> >>>> My plan was that once the plugin was added to VMMaker-Plugins that I
>>> >>>> would test it again on linux and windows, and then ask for the
>>> >>>> appropriate plugins.int to be updated.
>>> >>>>
>>> >>>>
>>> >>>> Thanks!
>>> >>>> Alistair
>>> >>>>
>>> >>>>
>>> >>>>> Thanks,
>>> >>>>>
>>> >>>>> --
>>> >>>>> Clément Béra
>>> >>>>> https://clementbera.wordpress.com/
>>> >>>>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
>>
>>
>>
>>
>> --
>> _,,,^..^,,,_
>> best, Eliot
>>
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

Eliot Miranda-2
 
Hi Alistair,

On Sat, Dec 23, 2017 at 9:35 AM, Alistair Grant <[hidden email]> wrote:

Hi Eliot,

I've made progress:

- Updated FileAttributesPlugin to use #primitiveFailForOSError:.
- Loaded PrimitiveOSError
- Updated SmalltalkImage>>newSpecialObjectsArray
- Run SmalltalkImage current recreateSpecialObjectsArray.
- And triggered an error which generated the appropriate
PrimitiveOSError object.

Given I've already made changes, and have a bit more tidying up to do,
it is probably isn't worthwhile you reviewing the code again until I
post an updated version.  I'll let you know when it is ready.

OK, that's great.  Thanks and have a great holiday!
 

Merry Christmas!
Alistair




On 22 December 2017 at 21:08, Alistair Grant <[hidden email]> wrote:
> Hi Eliot,
>
> Thanks!  I'd figured out the slang changes, I'll start work on the
> Smalltalk code changes after I've fixed my vm build system.
>
> Cheers,
> Alistair
>
>
> On 22 December 2017 at 20:59, Eliot Miranda <[hidden email]> wrote:
>>
>> Hi Alistair,  Hi Clément,
>>
>>   here are the code and an example of the new operating system error primitive fail facility.
>>
>> Here's the extract from recreateSpecialObjectsArray that adds the prototype to the table:
>>
>> newArray at: 52 put: #(nil "nil => generic error" #'bad receiver'
>> #'bad argument' #'bad index'
>> #'bad number of arguments'
>> #'inappropriate operation'  #'unsupported operation'
>> #'no modification' #'insufficient object memory'
>> #'insufficient C memory' #'not found' #'bad method'
>> #'internal error in named primitive machinery'
>> #'object may move' #'resource limit exceeded'
>> #'object is pinned' #'primitive write beyond end of object'
>> #'object moved' #'object not pinned' #'callback error'),
>> {PrimitiveOSError new errorName: #'operating system error'; yourself}.
>>
>> Here's the class definition (also attached) for PrimitiveOSError:
>>
>> !Object methodsFor: '*System-Support-error handling' stamp: 'eem 12/12/2017 09:16'!
>> isPrimitiveOSError
>> ^false! !
>>
>> Object subclass: #PrimitiveOSError
>> instanceVariableNames: 'errorName errorCode'
>> classVariableNames: ''
>> poolDictionaries: ''
>> category: 'System-Support'!
>> !PrimitiveOSError commentStamp: 'eem 12/7/2017 19:31' prior: 0!
>> A PrimitiveOSError is used to answer a primitive failure code that has an associated operating system/library error.
>>
>> Instance Variables
>> errorName: <Symbol>
>> errorValue: <Integer>
>>
>> errorName
>> - typically #'operating system error'
>>
>> errorValue
>> - the value of the error, a signed 64-bit value, a representation imposed by the VM; specific clients must map this error value into an unsigned value as appropriate if required!
>>
>>
>> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 19:32'!
>> errorCode
>> ^errorCode! !
>>
>> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 19:32'!
>> errorCode: anObject
>> errorCode := anObject! !
>>
>> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 18:16'!
>> errorName
>> ^errorName! !
>>
>> !PrimitiveOSError methodsFor: 'accessing' stamp: 'eem 12/7/2017 18:16'!
>> errorName: anObject
>> errorName := anObject! !
>>
>>
>> !PrimitiveOSError methodsFor: 'testing' stamp: 'eem 12/12/2017 09:15'!
>> isPrimitiveOSError
>> ^true! !
>>
>>
>> The signature of primitiveFailForOSError: is:
>> sqInt primitiveFailForOSError(sqLong)
>>
>> So in your Slang code use
>>
>>     interpreterProxy primitiveFailForOSError: osErrorCode
>>
>> or in your platform C code use
>>
>>
>>     interpreterProxy->primitiveFailForOSError(osErrorCode)
>>
>> In your Smalltalk code write things like
>>
>>      <primitive: 'foo' module: '' error: ec>
>>      ec isOSErrorCode ifTrue:
>>          [self processError: ec errorCode]
>>
>>
>>
>>
>> On Fri, Dec 22, 2017 at 9:11 AM, Alistair Grant <[hidden email]> wrote:
>>>
>>>
>>> Hi Eliot,
>>>
>>> On 22 December 2017 at 18:00, Eliot Miranda <[hidden email]> wrote:
>>> >
>>> > Hi Alistair,
>>> >
>>> >
>>> >> On Dec 22, 2017, at 8:33 AM, Alistair Grant <[hidden email]> wrote:
>>> >>
>>> >>
>>> >> Hi Eliot,
>>> >>
>>> >>> On 22 December 2017 at 17:09, Eliot Miranda <[hidden email]> wrote:
>>> >>>
>>> >>> Hi Clément, Hi Alistair,
>>> >>>
>>> >>>    FileAttrbutesPlugin is not yet ready for use.  The main problem is that it neither fails properly nor communicates error codes back properly.  Basically FileAttrbutesPlugin confuses failing because it gets invalid arguments with reporting error codes due to operating system level errors.  For the FileAttrbutesPlugin's primitives to work in Spur they must fail in the traditional way when given invalid arguments.  This is because the Spur VM will check when a primitive fails if there are any forwarders in the primitive's parameters, and if so, follow (fix up) the forwarders and retry the primitive.
>>> >>
>>> >> Is this the feedback you gave me on 2 September, or something different?
>>> >
>>> > One and the same.
>>> >
>>> >>
>>> >> Back then, you wrote:
>>> >>> The change that is required to error handling:
>>> >>> If a primitive fails due to argument marshaling it must use the primitiveFailedWith: mechanism and report the error using one of the error codes.  This is because Spur uses lazy forwarding to implement become:, pin, et al.  The lazy forwarding mechanism follows forwarders in the VM when they are encountered.  For sends this is trivial; any send to a forwarder fails and so the failure side of message lookup follows forwarders and retries the send.  Similarly for the arguments of primitives, when a primitive fails the VM searches for forwarders in the receiver and arguments to a specific pre-computed depth, following any forwarders it finds.  If any forwarders are found the primitive is retried after following all the forwarders found.
>>> >>
>>> >>
>>> >> My understanding is that this has already been done.  If I've
>>> >> misunderstood, my apologies.
>>> >
>>> > I should apologize.  One side effect of answering email in my phone is that I don't look things up as often as I should (blush).  But answering email in bed in the early morning while the kids are still asleep is irresistible.  Anyway, good.  I shall try and review the code this weekend or early next week.  Feel free to nag me.  Feel free to use harsh language ;-)
>>>
>>> No problem.    The email subject where I run over the mods is "Re:
>>> 20294 Add FileAttributesPlugin to the linux 32 bit VM", sent 6 Sep
>>> 2017.
>>>
>>>
>>>
>>> >>> Next, communication no os errors back through the primitives results is acceptable, but a better way would be to use the new #'operating system error' primitive failure code mechanism.  Here, provided a suitable two slot prototype is installed in the primitiveErrorTable (see Squeak trunk) then a plugin can do
>>> >>>        interpreterProxy primitiveFailForOSError(signedSixtyFourBitErrorCode)
>>> >>> and the VM will clone the error object whose first slot should be #'operating system error' and whose second slot will be the code.
>>> >>>
>>> >>> This gives us a clean way of communicating errors back to a primitive's method body in the traditional way.
>>> >>
>>> >> At the moment I'm passing back an error number generated by the
>>> >> plugin, not an OS error number returned by a system call, but I assume
>>> >> that the VM doesn't really care.
>>> >>
>>> >> Do you have an example where this is being used in a plugin?
>>> >
>>> > Not yet.  This is new.  Monty wanted if god the FilePlugin and we really needed it.  I'll prepare a proper email with examples of usage and a class def for the error prototype soon.  Again, harsh language may be effective ;-)
>>>
>>> OK.  I'll have a look, but the examples will be good.
>>>
>>> Thanks,
>>> Alistair
>>>
>>>
>>>
>>> >> Cheers,
>>> >> Alistair
>>> >
>>> > Cheers!
>>> >
>>> >>
>>> >>
>>> >>> I do apologize if I hadn't made this clear earlier.  But AFAIA there is significant work to do before the plugin is ready for integration.
>>> >>>
>>> >>> _,,,^..^,,,_ (phone)
>>> >>>
>>> >>>> On Dec 22, 2017, at 2:48 AM, Alistair Grant <[hidden email]> wrote:
>>> >>>>
>>> >>>>
>>> >>>> Hi Clément,
>>> >>>>
>>> >>>>> On 22 December 2017 at 11:29, Clément Bera <[hidden email]> wrote:
>>> >>>>>
>>> >>>>> Hi,
>>> >>>>>
>>> >>>>> What is the status of the FilesAttributesPlugin ?
>>> >>>>
>>> >>>> I'm hoping that it will be integrated soon.
>>> >>>>
>>> >>>>
>>> >>>>> I would like to make it available by default on pharo VMs.
>>> >>>>
>>> >>>> Yes, please. :-)
>>> >>>>
>>> >>>>
>>> >>>>> Questions:
>>> >>>>>
>>> >>>>> 1) Is it already merged with another plugin ?
>>> >>>>
>>> >>>> I'm not sure that I understand the question.  FileAttributesPlugin
>>> >>>> works along side FilePlugin (which is obviously already part of the
>>> >>>> pharo VM).
>>> >>>>
>>> >>>>
>>> >>>>> 2) If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
>>> >>>>
>>> >>>> That's my guess, but someone knowledgable needs to answer this.
>>> >>>>
>>> >>>>
>>> >>>>> 3) What is the right way to generate a plugin ? I use this:
>>> >>>>>
>>> >>>>> (VMMaker
>>> >>>>> makerFor: StackInterpreter
>>> >>>>> and: nil
>>> >>>>> with: #()
>>> >>>>> to: (FileDirectory default pathFromURI: '../src')
>>> >>>>> platformDir: (FileDirectory default pathFromURI: '../platforms')
>>> >>>>> including: #(FileAttributesPlugin)
>>> >>>>> ) generateExternalPlugin: FileAttributesPlugin
>>> >>>>>
>>> >>>>> Is that correct ?
>>> >>>>> It seems it generates only the C file but no header file.
>>> >>>>
>>> >>>> There isn't a FileAttributesPlugin.h.  It does need the platform support file:
>>> >>>>
>>> >>>> platforms/win32/plugins/FileAttributesPlugin/Makefile.plugin
>>> >>>>
>>> >>>> which is already part of opensmalltalk-vm.
>>> >>>>
>>> >>>> I've always used VMMakerTool to make the plugin, but if it's
>>> >>>> generating the .c file it's probably correct.
>>> >>>>
>>> >>>>
>>> >>>> My plan was that once the plugin was added to VMMaker-Plugins that I
>>> >>>> would test it again on linux and windows, and then ask for the
>>> >>>> appropriate plugins.int to be updated.
>>> >>>>
>>> >>>>
>>> >>>> Thanks!
>>> >>>> Alistair
>>> >>>>
>>> >>>>
>>> >>>>> Thanks,
>>> >>>>>
>>> >>>>> --
>>> >>>>> Clément Béra
>>> >>>>> https://clementbera.wordpress.com/
>>> >>>>> Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
>>
>>
>>
>>
>> --
>> _,,,^..^,,,_
>> best, Eliot
>>



--
_,,,^..^,,,_
best, Eliot
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

alistairgrant
 
Hi Eliot,

I've posted a new version of the FileAttributesPlugin
(FileAttributesPlugin.oscog-AlistairGrant.19):

http://smalltalkhub.com/#!/~Alistair/FileAttributesPlugin/versions/FileAttributesPlugin.oscog-AlistairGrant.19


Changes:

- Convert error values from coded values to #primitiveFailForOSError:
- #primitiveClosedir & #primitiveRewinddir return PrimErrBadArgument
  instead of a coded error for a bad argument


The updated Pharo code to use the modified plugin is in github:

https://github.com/akgrant43/FileAttributes

Would you please review the plugin code again (taking in to account the
feedback I sent in "Re: 20294 Add FileAttributesPlugin to the linux 32
bit VM", sent 6 Sep 2017).


Thanks!
Alistair



On 23 December 2017 at 19:00, Eliot Miranda <[hidden email]> wrote:

>
> Hi Alistair,
>
> On Sat, Dec 23, 2017 at 9:35 AM, Alistair Grant <[hidden email]> wrote:
>>
>>
>> Hi Eliot,
>>
>> I've made progress:
>>
>> - Updated FileAttributesPlugin to use #primitiveFailForOSError:.
>> - Loaded PrimitiveOSError
>> - Updated SmalltalkImage>>newSpecialObjectsArray
>> - Run SmalltalkImage current recreateSpecialObjectsArray.
>> - And triggered an error which generated the appropriate
>> PrimitiveOSError object.
>>
>> Given I've already made changes, and have a bit more tidying up to do,
>> it is probably isn't worthwhile you reviewing the code again until I
>> post an updated version.  I'll let you know when it is ready.
>
>
> OK, that's great.  Thanks and have a great holiday!
>
>>
>>
>> Merry Christmas!
>> Alistair
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

Eliot Miranda-2
 
Hi Alistair,

On Mon, Jan 1, 2018 at 12:40 AM, Alistair Grant <[hidden email]> wrote:

Hi Eliot,

I've posted a new version of the FileAttributesPlugin
(FileAttributesPlugin.oscog-AlistairGrant.19):

http://smalltalkhub.com/#!/~Alistair/FileAttributesPlugin/versions/FileAttributesPlugin.oscog-AlistairGrant.19

Looking good!  I have some minor quibbles, but so far see nothing major wrong.

So...

- the most important is not to query the SecurityPlugin more than once.  So follow the pattern in FilePlugin where the inst vars such as sCOFfn are set up in initialiseModule.  You're querying the SecurityPlugin on every access as in

canStatFilePath: aPathCString length: length
...
(hasSecurityPlugin = 0) ifTrue: [^ true].
sCOFfn := interpreterProxy ioLoadFunction: 'secCanOpenFileOfSizeWritable' From: 'SecurityPlugin'.
sCOFfn ~= 0
...

These are minor:

- accessAttributesForFilename:into:startingAt: doesn't need the remapOop:in:.  No instantiations occur so the GC will never kick in.
- primitives such as primitiveClosedir that use pointerFor: don't need to check the argument via is: dirPointerOop KindOf: 'ByteArray'; pointerFor: does all the checking one needs.
- in primitives such as primitiveFileExists I would rewrite

accessFlag = 0
ifTrue: [interpreterProxy pop: 2 thenPush: interpreterProxy trueObject]
ifFalse: [interpreterProxy pop: 2 thenPush: interpreterProxy falseObject]

  as

interpreterProxy pop: 2 thenPush: (accessFlag = 0
ifTrue: [iinterpreterProxy trueObject]
ifFalse: [interpreterProxy falseObject])

- given that you're accessing W_OK et al via Symbols (see #W_OK et al in accessAttributesForFilename:into:startingAt:) do you need fileWriteableFlag et al?  Or should you use them instead of the Symbols?  The latter is easier when we have to simulate the plugin when debugging the VM at some future date.

- there are still some users of cPreprocessorDirective:.  It is much more preferable to use cppIf:ifTrue:[ifFalse:]


Changes:

- Convert error values from coded values to #primitiveFailForOSError:
- #primitiveClosedir & #primitiveRewinddir return PrimErrBadArgument
  instead of a coded error for a bad argument


The updated Pharo code to use the modified plugin is in github:

https://github.com/akgrant43/FileAttributes

Would you please review the plugin code again (taking in to account the
feedback I sent in "Re: 20294 Add FileAttributesPlugin to the linux 32
bit VM", sent 6 Sep 2017).


Thanks!
Alistair



On 23 December 2017 at 19:00, Eliot Miranda <[hidden email]> wrote:
>
> Hi Alistair,
>
> On Sat, Dec 23, 2017 at 9:35 AM, Alistair Grant <[hidden email]> wrote:
>>
>>
>> Hi Eliot,
>>
>> I've made progress:
>>
>> - Updated FileAttributesPlugin to use #primitiveFailForOSError:.
>> - Loaded PrimitiveOSError
>> - Updated SmalltalkImage>>newSpecialObjectsArray
>> - Run SmalltalkImage current recreateSpecialObjectsArray.
>> - And triggered an error which generated the appropriate
>> PrimitiveOSError object.
>>
>> Given I've already made changes, and have a bit more tidying up to do,
>> it is probably isn't worthwhile you reviewing the code again until I
>> post an updated version.  I'll let you know when it is ready.
>
>
> OK, that's great.  Thanks and have a great holiday!
>
>>
>>
>> Merry Christmas!
>> Alistair



--
_,,,^..^,,,_
best, Eliot
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

Jakob Reschke
 

Am 02.01.2018 04:43 schrieb "Eliot Miranda" <[hidden email]>:
 ... inst vars such as sCOFfn are set up in initialiseModule.  You're querying the SecurityPlugin on every access as in

canStatFilePath: aPathCString length: length
...
(hasSecurityPlugin = 0) ifTrue: [^ true].
sCOFfn := interpreterProxy ioLoadFunction: 'secCanOpenFileOfSizeWritable' From: 'SecurityPlugin'.
sCOFfn ~= 0
...

I have never looked at this before, so: can this variable name be spelled out, please, so it becomes legible for the uninitiated folk?
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

alistairgrant
In reply to this post by Eliot Miranda-2
 
Hi Eliot,

Thanks for your feedback!

TL;DR: I've made all the changes you suggested below (more verbose
response below my signature).

FileAttributesPlugin.oscog-AlistairGrant.20 is now available at:

http://smalltalkhub.com/#!/~Alistair/FileAttributesPlugin/versions/FileAttributesPlugin.oscog-AlistairGrant.20

I'm not mentioning this every time, but there's about 540 automated
tests related to the file system, which I confirm that all pass before
posting updates.  I also use the plugin in my development environment.


Thanks again,
Alistair


On Mon, Jan 01, 2018 at 07:42:38PM -0800, Eliot Miranda wrote:

>  
> Hi Alistair,
>
> On Mon, Jan 1, 2018 at 12:40 AM, Alistair Grant <[hidden email]> wrote:
>
>
>     Hi Eliot,
>
>     I've posted a new version of the FileAttributesPlugin
>     (FileAttributesPlugin.oscog-AlistairGrant.19):
>
>     http://smalltalkhub.com/#!/~Alistair/FileAttributesPlugin/versions/
>     FileAttributesPlugin.oscog-AlistairGrant.19
>
>
> Looking good!  I have some minor quibbles, but so far see nothing major wrong.
>
> So...
>
> - the most important is not to query the SecurityPlugin more than once.  So
> follow the pattern in FilePlugin where the inst vars such as sCOFfn are set up
> in initialiseModule.  You're querying the SecurityPlugin on every access as in
>
> canStatFilePath: aPathCString length: length
> ...
> (hasSecurityPlugin = 0) ifTrue: [^ true].
> sCOFfn := interpreterProxy ioLoadFunction: 'secCanOpenFileOfSizeWritable' From:
> 'SecurityPlugin'.
> sCOFfn ~= 0
> ...

Fixed.


> These are minor:
>
> - accessAttributesForFilename:into:startingAt: doesn't need the remapOop:in:.
> No instantiations occur so the GC will never kick in.

Done.


> - primitives such as primitiveClosedir that use pointerFor: don't need to check
> the argument via is: dirPointerOop KindOf: 'ByteArray'; pointerFor: does all
> the checking one needs.

Done.


> - in primitives such as primitiveFileExists I would rewrite
>
> accessFlag = 0
> ifTrue: [interpreterProxy pop: 2 thenPush: interpreterProxy trueObject]
> ifFalse: [interpreterProxy pop: 2 thenPush: interpreterProxy falseObject]
>
>   as
>
> interpreterProxy pop: 2 thenPush: (accessFlag = 0
> ifTrue: [iinterpreterProxy trueObject]
> ifFalse: [interpreterProxy falseObject])

This follows from my self-talk: "If accessFlag is 0, then push true,
otherwise, push false".

But I've changed them all anyway.


> - given that you're accessing W_OK et al via Symbols (see #W_OK et al in
> accessAttributesForFilename:into:startingAt:) do you need fileWriteableFlag et
> al?  Or should you use them instead of the Symbols?  The latter is easier when
> we have to simulate the plugin when debugging the VM at some future date.

I've changed them all to use the accessor so that the simulator is
easier later on.


> - there are still some users of cPreprocessorDirective:.  It is much more
> preferable to use cppIf:ifTrue:[ifFalse:]

Last time I tried this it generated invalid C code.  I think I know what
I did wrong now, so #cPreprocessorDirective: has been replaced.  

Cheers,
Alistair

Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

alistairgrant
In reply to this post by Jakob Reschke
 
Hi Jakob,

On 2 January 2018 at 06:46, Jakob Reschke <[hidden email]> wrote:

>
>
> Am 02.01.2018 04:43 schrieb "Eliot Miranda" <[hidden email]>:
>
>  ... inst vars such as sCOFfn are set up in initialiseModule.  You're querying the SecurityPlugin on every access as in
>
>
> canStatFilePath: aPathCString length: length
> ...
> (hasSecurityPlugin = 0) ifTrue: [^ true].
> sCOFfn := interpreterProxy ioLoadFunction: 'secCanOpenFileOfSizeWritable' From: 'SecurityPlugin'.
> sCOFfn ~= 0
> ...
>
>
> I have never looked at this before, so: can this variable name be spelled out, please, so it becomes legible for the uninitiated folk?

I've made this change in my image, so it will flow through on the next update.

Cheers,
Alistair
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

timrowledge
In reply to this post by Jakob Reschke
 

> On 01-01-2018, at 9:46 PM, Jakob Reschke <[hidden email]> wrote:
>
>
> Am 02.01.2018 04:43 schrieb "Eliot Miranda" <[hidden email]>:
>  ... inst vars such as sCOFfn are set up in initialiseModule.  You're querying the SecurityPlugin on every access as in
>
> canStatFilePath: aPathCString length: length
> ...
> (hasSecurityPlugin = 0) ifTrue: [^ true].
> sCOFfn := interpreterProxy ioLoadFunction: 'secCanOpenFileOfSizeWritable' From: 'SecurityPlugin'.
> sCOFfn ~= 0
> ...
>
> I have never looked at this before, so: can this variable name be spelled out, please, so it becomes legible for the uninitiated folk?

It’s just the initials of the name of the function it points to :-)

tim
--
tim Rowledge; [hidden email]; http://www.rowledge.org/tim
Klingon Code Warrior:- 4) "This machine is a piece of GAGH! I need dual A11bioniq processors if I am to do battle with  this code!"


Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

Eliot Miranda-2
In reply to this post by alistairgrant
 
Hi Alistair,

On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant <[hidden email]> wrote:

Hi Jakob,

On 2 January 2018 at 06:46, Jakob Reschke <[hidden email]> wrote:
>
>
> Am 02.01.2018 04:43 schrieb "Eliot Miranda" <[hidden email]>:
>
>  ... inst vars such as sCOFfn are set up in initialiseModule.  You're querying the SecurityPlugin on every access as in
>
>
> canStatFilePath: aPathCString length: length
> ...
> (hasSecurityPlugin = 0) ifTrue: [^ true].
> sCOFfn := interpreterProxy ioLoadFunction: 'secCanOpenFileOfSizeWritable' From: 'SecurityPlugin'.
> sCOFfn ~= 0
> ...
>
>
> I have never looked at this before, so: can this variable name be spelled out, please, so it becomes legible for the uninitiated folk?

I've made this change in my image, so it will flow through on the next update.

I don't think it's necessary.  The long vars are unwieldy and I'm sure Jakob understands the convention now.

I just did a minor edit on the plain.  I wonder if it's time to move the package to the VMMaker repository.  In any case I'll start including it in builds.

_,,,^..^,,,_
best, Eliot
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: Merging FilesAttributesPlugin

Jakob Reschke
 
2018-01-03 19:04 GMT+01:00 Eliot Miranda <[hidden email]>:

> On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant <[hidden email]> wrote:
>> On 2 January 2018 at 06:46, Jakob Reschke <[hidden email]> wrote
>> > Am 02.01.2018 04:43 schrieb "Eliot Miranda" <[hidden email]>:
>> >
>> >  ... inst vars such as sCOFfn are set up in initialiseModule.  You're querying the SecurityPlugin on every access as in
>> >
>> > canStatFilePath: aPathCString length: length
>> > ...
>> > (hasSecurityPlugin = 0) ifTrue: [^ true].
>> > sCOFfn := interpreterProxy ioLoadFunction: 'secCanOpenFileOfSizeWritable' From: 'SecurityPlugin'.
>> > sCOFfn ~= 0
>> > ...
>> >
>> > I have never looked at this before, so: can this variable name be spelled out, please, so it becomes legible for the uninitiated folk?
>>
>> I've made this change in my image, so it will flow through on the next update.
>
> I don't think it's necessary.  The long vars are unwieldy and I'm sure Jakob understands the convention now.
>

Actually I did figure this out myself, but nevertheless I think it
unnecessarily complicates reading the code when things are
abbreviated. Even if you already know/knew what it stands for. In the
case of "sCOF", I would probably have to look the meaning up again
every time I return to that code after a few months, if I were to do
that.

Sure, this is a matter of style preferences, but I consider it
suboptimal if you have to think harder to understand something when it
could also have been made clear and easy at writing time (without
sacrificing functionality and performance too much, of course). Please
spare the newcomer readers some "wtf" moments if you can. ;-)
Especially for things like inst vars, whose initialization (and
explanation) may be far away from their usage.

Kind regards,
Jakob
12