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,
Hi Clément,
On 22 December 2017 at 11:29, Clément Bera bera.clement@gmail.com 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:
- 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).
- 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.
- 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
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 akgrant0710@gmail.com wrote:
Hi Clément,
On 22 December 2017 at 11:29, Clément Bera bera.clement@gmail.com 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:
- 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).
- 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.
- 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
Hi Eliot,
On 22 December 2017 at 17:09, Eliot Miranda eliot.miranda@gmail.com 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 akgrant0710@gmail.com wrote:
Hi Clément,
On 22 December 2017 at 11:29, Clément Bera bera.clement@gmail.com 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:
- 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).
- 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.
- 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
Hi Alistair,
On Dec 22, 2017, at 8:33 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 22 December 2017 at 17:09, Eliot Miranda eliot.miranda@gmail.com 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 akgrant0710@gmail.com wrote:
Hi Clément,
On 22 December 2017 at 11:29, Clément Bera bera.clement@gmail.com 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:
- 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).
- 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.
- 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
Hi Eliot,
On 22 December 2017 at 18:00, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Dec 22, 2017, at 8:33 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 22 December 2017 at 17:09, Eliot Miranda eliot.miranda@gmail.com 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 akgrant0710@gmail.com wrote:
Hi Clément,
On 22 December 2017 at 11:29, Clément Bera bera.clement@gmail.com 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:
- 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).
- 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.
- 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
Hi Eliot,
On 22 December 2017 at 18:11, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 22 December 2017 at 18:00, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Dec 22, 2017, at 8:33 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 22 December 2017 at 17:09, Eliot Miranda eliot.miranda@gmail.com 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 akgrant0710@gmail.com wrote:
Hi Clément,
On 22 December 2017 at 11:29, Clément Bera bera.clement@gmail.com 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:
- 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).
- 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.
- 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
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 akgrant0710@gmail.com wrote:
Hi Eliot,
On 22 December 2017 at 18:00, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Dec 22, 2017, at 8:33 AM, Alistair Grant akgrant0710@gmail.com
wrote:
Hi Eliot,
On 22 December 2017 at 17:09, Eliot Miranda eliot.miranda@gmail.com
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 akgrant0710@gmail.com
wrote:
Hi Clément,
On 22 December 2017 at 11:29, Clément Bera bera.clement@gmail.com
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:
- 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).
- 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.
- 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
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 eliot.miranda@gmail.com 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: osErrorCodeor 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 akgrant0710@gmail.com wrote:
Hi Eliot,
On 22 December 2017 at 18:00, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Dec 22, 2017, at 8:33 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 22 December 2017 at 17:09, Eliot Miranda eliot.miranda@gmail.com 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 akgrant0710@gmail.com wrote:
Hi Clément,
> On 22 December 2017 at 11:29, Clément Bera bera.clement@gmail.com 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
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 akgrant0710@gmail.com 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 eliot.miranda@gmail.com 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: osErrorCodeor 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 akgrant0710@gmail.com wrote:
Hi Eliot,
On 22 December 2017 at 18:00, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Dec 22, 2017, at 8:33 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 22 December 2017 at 17:09, Eliot Miranda eliot.miranda@gmail.com 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 akgrant0710@gmail.com wrote: > > > Hi Clément, > >> On 22 December 2017 at 11:29, Clément Bera bera.clement@gmail.com 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
Hi Alistair,
On Sat, Dec 23, 2017 at 9:35 AM, Alistair Grant akgrant0710@gmail.com 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 akgrant0710@gmail.com 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 eliot.miranda@gmail.com
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: osErrorCodeor 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 akgrant0710@gmail.com
wrote:
Hi Eliot,
On 22 December 2017 at 18:00, Eliot Miranda eliot.miranda@gmail.com
wrote:
Hi Alistair,
On Dec 22, 2017, at 8:33 AM, Alistair Grant akgrant0710@gmail.com
wrote:
Hi Eliot,
> On 22 December 2017 at 17:09, Eliot Miranda <
eliot.miranda@gmail.com> 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 <
akgrant0710@gmail.com> wrote:
>> >> >> Hi Clément, >> >>> On 22 December 2017 at 11:29, Clément Bera <
bera.clement@gmail.com> 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
Hi Eliot,
I've posted a new version of the FileAttributesPlugin (FileAttributesPlugin.oscog-AlistairGrant.19):
http://smalltalkhub.com/#!/~Alistair/FileAttributesPlugin/versions/FileAttri...
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 eliot.miranda@gmail.com wrote:
Hi Alistair,
On Sat, Dec 23, 2017 at 9:35 AM, Alistair Grant akgrant0710@gmail.com 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
Hi Alistair,
On Mon, Jan 1, 2018 at 12:40 AM, Alistair Grant akgrant0710@gmail.com 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 eliot.miranda@gmail.com wrote:
Hi Alistair,
On Sat, Dec 23, 2017 at 9:35 AM, Alistair Grant akgrant0710@gmail.com
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
Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com:
... 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?
Hi Jakob,
On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote:
Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com:
... 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
Hi Alistair,
On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Jakob,
On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote:
Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com:
... 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
2018-01-03 19:04 GMT+01:00 Eliot Miranda eliot.miranda@gmail.com:
On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant akgrant0710@gmail.com wrote:
On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote
Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com:
... 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
Hi Eliot,
On 3 January 2018 at 19:04, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Jakob,
On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote:
Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com:
... 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.
OK.
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.
Yes, please :-)
My build system has suddenly decided that it can't find libpulse-simple. I can't see that any of the changes you've made will impact it, but I'll work on getting my build working again and running it through the test suite.
Thanks! Alistair
Hi Eliot,
On 3 January 2018 at 20:20, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 3 January 2018 at 19:04, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Jakob,
On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote:
Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com:
... 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.
OK.
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.
Yes, please :-)
My build system has suddenly decided that it can't find libpulse-simple. I can't see that any of the changes you've made will impact it, but I'll work on getting my build working again and running it through the test suite.
Deleting the running docker container and going back to the clean image fixed the libpulse-simple problem.
And all the automated tests pass (FileAttributesPlugin.oscog-eem.22, Ubuntu 16.04).
Thanks! Alistair
Hi Eliot,
I'm able to reliably crash the 64 bit VM by passing a string to primitiveClosedir in the FileAttributesPlugin (which should never be done in practice, I've got it as a test for the parameter checking):
primitiveClosedir "Close the directory stream for dirPointerOop. Answer dirPointerOop on success. Raise PrimErrBadArgument if the parameter is not a ByteArray length size(void *). If closedir() returns an error raise PrimitiveOSError."
| dirPointerOop dirStream result | <export: true> <var: 'dirStream' type: 'osdir *'> dirPointerOop := interpreterProxy stackValue: 0.
">>>> Putting the following back prevents the crash..." (interpreterProxy is: dirPointerOop KindOf: 'ByteArray') ifFalse: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. "<<<<"
dirStream := self pointerFrom: dirPointerOop. dirStream ifNil: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. result := self closedir: dirStream dp. result = 0 ifFalse: [^interpreterProxy primitiveFailForOSError: self unableToCloseDir]. self free: dirStream. interpreterProxy pop: 2 thenPush: dirPointerOop
I removed the explicit is:KindOf: ByteArray check as you said that the checks in pointerFrom: would be sufficient.
Would you mind letting me know what your preferred solution is (put the explicit ByteArray check back in, or look to fix the checks in pointerFrom:, or ...)?
Just for reference, pointerFrom: is:
pointerFrom: directoryPointerBytes "Answer the machine address contained in anExternalAddressOop."
| ptr addressUnion idx | <returnTypeC: 'void *'> <var: 'ptr' type: 'unsigned char *'> <var: 'addressUnion' type: 'union {void *address; unsigned char bytes[sizeof(void *)];}'> ((interpreterProxy isBytes: directoryPointerBytes) and: [(interpreterProxy stSizeOf: directoryPointerBytes) = self sizeOfPointer]) ifFalse: [^ nil]. ptr := interpreterProxy arrayValueOf: directoryPointerBytes. idx := 0. [idx < self sizeOfPointer] whileTrue: [self cCode: 'addressUnion.bytes[idx] = ptr[idx]'. idx := idx + 1]. ^ self cCode: 'addressUnion.address' inSmalltalk: [addressUnion]
Thanks, Alistair
On 3 January 2018 at 20:45, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 3 January 2018 at 20:20, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 3 January 2018 at 19:04, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Jakob,
On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote:
Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com:
... 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.
OK.
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.
Yes, please :-)
My build system has suddenly decided that it can't find libpulse-simple. I can't see that any of the changes you've made will impact it, but I'll work on getting my build working again and running it through the test suite.
Deleting the running docker container and going back to the clean image fixed the libpulse-simple problem.
And all the automated tests pass (FileAttributesPlugin.oscog-eem.22, Ubuntu 16.04).
Thanks! Alistair
Hi Eliot,
Yet another lesson (for me) on why I shouldn't rush emails out just before going to bed.
The crash is due to #pointerFrom: only checking for a byte type object, so sending a random string of the correct length will cause the VM to crash. I've tightened the parameter checking, and neither the 32 bit or 64 bit images crash now.
Updated version:
Name: FileAttributesPlugin.oscog-AlistairGrant.25 Author: AlistairGrant Time: 4 January 2018, 4:02:11.405148 pm UUID: 5b76e8bb-69e1-4db4-b529-9de1249e63e0 Ancestors: FileAttributesPlugin.oscog-eem.24
Version 1.2.3:
- Ensure that #pointerFrom: is given a ByteArray, not just a byte object, which can be a string.
Thanks, Alistair
On 4 January 2018 at 00:05, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
I'm able to reliably crash the 64 bit VM by passing a string to primitiveClosedir in the FileAttributesPlugin (which should never be done in practice, I've got it as a test for the parameter checking):
primitiveClosedir "Close the directory stream for dirPointerOop. Answer dirPointerOop on success. Raise PrimErrBadArgument if the parameter is not a ByteArray length size(void *). If closedir() returns an error raise PrimitiveOSError."
| dirPointerOop dirStream result | <export: true> <var: 'dirStream' type: 'osdir *'> dirPointerOop := interpreterProxy stackValue: 0. ">>>> Putting the following back prevents the crash..." (interpreterProxy is: dirPointerOop KindOf: 'ByteArray') ifFalse: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. "<<<<" dirStream := self pointerFrom: dirPointerOop. dirStream ifNil: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. result := self closedir: dirStream dp. result = 0 ifFalse: [^interpreterProxy primitiveFailForOSError: self unableToCloseDir]. self free: dirStream. interpreterProxy pop: 2 thenPush: dirPointerOopI removed the explicit is:KindOf: ByteArray check as you said that the checks in pointerFrom: would be sufficient.
Would you mind letting me know what your preferred solution is (put the explicit ByteArray check back in, or look to fix the checks in pointerFrom:, or ...)?
Just for reference, pointerFrom: is:
pointerFrom: directoryPointerBytes "Answer the machine address contained in anExternalAddressOop."
| ptr addressUnion idx | <returnTypeC: 'void *'> <var: 'ptr' type: 'unsigned char *'> <var: 'addressUnion' type: 'union {void *address; unsigned charbytes[sizeof(void *)];}'> ((interpreterProxy isBytes: directoryPointerBytes) and: [(interpreterProxy stSizeOf: directoryPointerBytes) = self sizeOfPointer]) ifFalse: [^ nil]. ptr := interpreterProxy arrayValueOf: directoryPointerBytes. idx := 0. [idx < self sizeOfPointer] whileTrue: [self cCode: 'addressUnion.bytes[idx] = ptr[idx]'. idx := idx + 1]. ^ self cCode: 'addressUnion.address' inSmalltalk: [addressUnion]
Thanks, Alistair
On 3 January 2018 at 20:45, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 3 January 2018 at 20:20, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 3 January 2018 at 19:04, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Jakob,
On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote:
Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com:
... 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.
OK.
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.
Yes, please :-)
My build system has suddenly decided that it can't find libpulse-simple. I can't see that any of the changes you've made will impact it, but I'll work on getting my build working again and running it through the test suite.
Deleting the running docker container and going back to the clean image fixed the libpulse-simple problem.
And all the automated tests pass (FileAttributesPlugin.oscog-eem.22, Ubuntu 16.04).
Thanks! Alistair
Hi Alistair,
On Jan 4, 2018, at 7:06 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
Yet another lesson (for me) on why I shouldn't rush emails out just before going to bed.
The crash is due to #pointerFrom: only checking for a byte type object, so sending a random string of the correct length will cause the VM to crash. I've tightened the parameter checking, and neither the 32 bit or 64 bit images crash now.
So was the problem that you passed in a string as the argument for one of the directory enumeration primitives and that the primitive interpreter the string as a pointer to a direct struct?
If so consider this redesign: The result of the opendir primitive is a pinned ByteArray that is the entire direct struct, ideally with a session id either prepended or appended. There is then a very small possibility of passing in some malformed argument, and even if one did manage to spoof the primitive's session id validity check the primitive wouldn't crash, but the directory support routines might, and hopefully would answer an error.
You can then eliminate malloc/free usage and leave it to the GC to collect the dirent ByteArrays.
Updated version:
Name: FileAttributesPlugin.oscog-AlistairGrant.25 Author: AlistairGrant Time: 4 January 2018, 4:02:11.405148 pm UUID: 5b76e8bb-69e1-4db4-b529-9de1249e63e0 Ancestors: FileAttributesPlugin.oscog-eem.24
Version 1.2.3:
- Ensure that #pointerFrom: is given a ByteArray, not just a byte
object, which can be a string.
Thanks, Alistair
On 4 January 2018 at 00:05, Alistair Grant akgrant0710@gmail.com wrote: Hi Eliot,
I'm able to reliably crash the 64 bit VM by passing a string to primitiveClosedir in the FileAttributesPlugin (which should never be done in practice, I've got it as a test for the parameter checking):
primitiveClosedir "Close the directory stream for dirPointerOop. Answer dirPointerOop on success. Raise PrimErrBadArgument if the parameter is not a ByteArray length size(void *). If closedir() returns an error raise PrimitiveOSError."
| dirPointerOop dirStream result | <export: true> <var: 'dirStream' type: 'osdir *'> dirPointerOop := interpreterProxy stackValue: 0.
">>>> Putting the following back prevents the crash..." (interpreterProxy is: dirPointerOop KindOf: 'ByteArray') ifFalse: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. "<<<<"
dirStream := self pointerFrom: dirPointerOop. dirStream ifNil: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. result := self closedir: dirStream dp. result = 0 ifFalse: [^interpreterProxy primitiveFailForOSError: self unableToCloseDir]. self free: dirStream. interpreterProxy pop: 2 thenPush: dirPointerOop
I removed the explicit is:KindOf: ByteArray check as you said that the checks in pointerFrom: would be sufficient.
Would you mind letting me know what your preferred solution is (put the explicit ByteArray check back in, or look to fix the checks in pointerFrom:, or ...)?
Just for reference, pointerFrom: is:
pointerFrom: directoryPointerBytes "Answer the machine address contained in anExternalAddressOop."
| ptr addressUnion idx | <returnTypeC: 'void *'> <var: 'ptr' type: 'unsigned char *'> <var: 'addressUnion' type: 'union {void *address; unsigned char bytes[sizeof(void *)];}'> ((interpreterProxy isBytes: directoryPointerBytes) and: [(interpreterProxy stSizeOf: directoryPointerBytes) = self sizeOfPointer]) ifFalse: [^ nil]. ptr := interpreterProxy arrayValueOf: directoryPointerBytes. idx := 0. [idx < self sizeOfPointer] whileTrue: [self cCode: 'addressUnion.bytes[idx] = ptr[idx]'. idx := idx + 1]. ^ self cCode: 'addressUnion.address' inSmalltalk: [addressUnion]
Thanks, Alistair
On 3 January 2018 at 20:45, Alistair Grant akgrant0710@gmail.com wrote: Hi Eliot,
On 3 January 2018 at 20:20, Alistair Grant akgrant0710@gmail.com wrote: Hi Eliot,
On 3 January 2018 at 19:04, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Jakob,
> On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote: > > > Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com: > > ... 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.
OK.
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.
Yes, please :-)
My build system has suddenly decided that it can't find libpulse-simple. I can't see that any of the changes you've made will impact it, but I'll work on getting my build working again and running it through the test suite.
Deleting the running docker container and going back to the clean image fixed the libpulse-simple problem.
And all the automated tests pass (FileAttributesPlugin.oscog-eem.22, Ubuntu 16.04).
Thanks! Alistair
Hi Eliot,
On 4 January 2018 at 21:25, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Jan 4, 2018, at 7:06 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
Yet another lesson (for me) on why I shouldn't rush emails out just before going to bed.
The crash is due to #pointerFrom: only checking for a byte type object, so sending a random string of the correct length will cause the VM to crash. I've tightened the parameter checking, and neither the 32 bit or 64 bit images crash now.
So was the problem that you passed in a string as the argument for one of the directory enumeration primitives and that the primitive interpreter the string as a pointer to a direct struct?
If so consider this redesign: The result of the opendir primitive is a pinned ByteArray that is the entire direct struct, ideally with a session id either prepended or appended. There is then a very small possibility of passing in some malformed argument, and even if one did manage to spoof the primitive's session id validity check the primitive wouldn't crash, but the directory support routines might, and hopefully would answer an error.
You can then eliminate malloc/free usage and leave it to the GC to collect the dirent ByteArrays.
The current design (copied from David's DirectoryPlugin) is that the pointer returned by opendir() is saved in a ByteArray (there's nothing special about this object, it isn't pinned as far as I know, and no malloc/free calls involved :-)). The pointer is then retrieved and used in subsequent operations. Being C, if a bad pointer is passed in to readdir() or closedir() the result is a core dump. So to minimise the chance of that happening I'm making the parameter checking as strict as possible, and ensuring that a string (of the correct length) isn't passed in by accident. As you say, the chances of that happening are tiny, but the results are severe (a core dump). It appears to be working correctly in the latest version.
I've also rebuilt the plugin on Windows 32bit and had to make a few more changes:
- The combination of #cppIf:ifTrue:[ifFalse:] and more agressive inlining means that smalltalk #ifTrue:#ifFalse: message sends are translated to C ternary conditional statements (?:). If the smalltalk #ifTrue:#ifFalse: contains a return (^), this can end up generating invalid C code, sometimes trying to return from within the ternary conditional, and sometimes trying to goto out of the conditional. - There was a bug in the time zone handling on Windows, which I've fixed.
The latest version is now:
Name: FileAttributesPlugin.oscog-AlistairGrant.26 Author: AlistairGrant Time: 5 January 2018, 9:02:46.033079 am UUID: 07c4e352-8193-452a-a80a-440f0811d43f Ancestors: FileAttributesPlugin.oscog-AlistairGrant.25
Version 1.2.4:
- Adjust for #cppIf:ifTrue:[ifFalse:] and inlining behaviour -- Smalltalk #ifTrue:#ifFalse: messages appear to be translated to C ternary operator instead of a if[else] statement. - Remove call to sqFileInit() (accidentally copied across from FilePlugin). - Fix stat() time zone adjustments on WIN32
Thanks! Alistair
Updated version:
Name: FileAttributesPlugin.oscog-AlistairGrant.25 Author: AlistairGrant Time: 4 January 2018, 4:02:11.405148 pm UUID: 5b76e8bb-69e1-4db4-b529-9de1249e63e0 Ancestors: FileAttributesPlugin.oscog-eem.24
Version 1.2.3:
- Ensure that #pointerFrom: is given a ByteArray, not just a byte
object, which can be a string.
Thanks, Alistair
On 4 January 2018 at 00:05, Alistair Grant akgrant0710@gmail.com wrote: Hi Eliot,
I'm able to reliably crash the 64 bit VM by passing a string to primitiveClosedir in the FileAttributesPlugin (which should never be done in practice, I've got it as a test for the parameter checking):
primitiveClosedir "Close the directory stream for dirPointerOop. Answer dirPointerOop on success. Raise PrimErrBadArgument if the parameter is not a ByteArray length size(void *). If closedir() returns an error raise PrimitiveOSError."
| dirPointerOop dirStream result | <export: true> <var: 'dirStream' type: 'osdir *'> dirPointerOop := interpreterProxy stackValue: 0.
">>>> Putting the following back prevents the crash..." (interpreterProxy is: dirPointerOop KindOf: 'ByteArray') ifFalse: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. "<<<<"
dirStream := self pointerFrom: dirPointerOop. dirStream ifNil: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. result := self closedir: dirStream dp. result = 0 ifFalse: [^interpreterProxy primitiveFailForOSError: self unableToCloseDir]. self free: dirStream. interpreterProxy pop: 2 thenPush: dirPointerOop
I removed the explicit is:KindOf: ByteArray check as you said that the checks in pointerFrom: would be sufficient.
Would you mind letting me know what your preferred solution is (put the explicit ByteArray check back in, or look to fix the checks in pointerFrom:, or ...)?
Just for reference, pointerFrom: is:
pointerFrom: directoryPointerBytes "Answer the machine address contained in anExternalAddressOop."
| ptr addressUnion idx | <returnTypeC: 'void *'> <var: 'ptr' type: 'unsigned char *'> <var: 'addressUnion' type: 'union {void *address; unsigned char bytes[sizeof(void *)];}'> ((interpreterProxy isBytes: directoryPointerBytes) and: [(interpreterProxy stSizeOf: directoryPointerBytes) = self sizeOfPointer]) ifFalse: [^ nil]. ptr := interpreterProxy arrayValueOf: directoryPointerBytes. idx := 0. [idx < self sizeOfPointer] whileTrue: [self cCode: 'addressUnion.bytes[idx] = ptr[idx]'. idx := idx + 1]. ^ self cCode: 'addressUnion.address' inSmalltalk: [addressUnion]
Thanks, Alistair
On 3 January 2018 at 20:45, Alistair Grant akgrant0710@gmail.com wrote: Hi Eliot,
On 3 January 2018 at 20:20, Alistair Grant akgrant0710@gmail.com wrote: Hi Eliot,
On 3 January 2018 at 19:04, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
> On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant akgrant0710@gmail.com wrote: > > > Hi Jakob, > >> On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote: >> >> >> Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com: >> >> ... 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.
OK.
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.
Yes, please :-)
My build system has suddenly decided that it can't find libpulse-simple. I can't see that any of the changes you've made will impact it, but I'll work on getting my build working again and running it through the test suite.
Deleting the running docker container and going back to the clean image fixed the libpulse-simple problem.
And all the automated tests pass (FileAttributesPlugin.oscog-eem.22, Ubuntu 16.04).
Thanks! Alistair
Hi Eliot,
Could I ask you to update FileAttributesPlugin.c on github in OpenSmalltalk/opensmalltalk-vm to:
Name: FileAttributesPlugin.oscog-AlistairGrant.26 Author: AlistairGrant Time: 5 January 2018, 9:02:46.033079 am UUID: 07c4e352-8193-452a-a80a-440f0811d43f Ancestors: FileAttributesPlugin.oscog-AlistairGrant.25
This version compiles on Windows and fixes a problem with DST handling on Windows.
http://smalltalkhub.com/#!/~Alistair/FileAttributesPlugin/versions/FileAttri...
If you'd prefer that I generate the C file and submit a PR, let me know.
Thanks! Alistair
On 5 January 2018 at 09:21, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
On 4 January 2018 at 21:25, Eliot Miranda eliot.miranda@gmail.com wrote:
Hi Alistair,
On Jan 4, 2018, at 7:06 AM, Alistair Grant akgrant0710@gmail.com wrote:
Hi Eliot,
Yet another lesson (for me) on why I shouldn't rush emails out just before going to bed.
The crash is due to #pointerFrom: only checking for a byte type object, so sending a random string of the correct length will cause the VM to crash. I've tightened the parameter checking, and neither the 32 bit or 64 bit images crash now.
So was the problem that you passed in a string as the argument for one of the directory enumeration primitives and that the primitive interpreter the string as a pointer to a direct struct?
If so consider this redesign: The result of the opendir primitive is a pinned ByteArray that is the entire direct struct, ideally with a session id either prepended or appended. There is then a very small possibility of passing in some malformed argument, and even if one did manage to spoof the primitive's session id validity check the primitive wouldn't crash, but the directory support routines might, and hopefully would answer an error.
You can then eliminate malloc/free usage and leave it to the GC to collect the dirent ByteArrays.
The current design (copied from David's DirectoryPlugin) is that the pointer returned by opendir() is saved in a ByteArray (there's nothing special about this object, it isn't pinned as far as I know, and no malloc/free calls involved :-)). The pointer is then retrieved and used in subsequent operations. Being C, if a bad pointer is passed in to readdir() or closedir() the result is a core dump. So to minimise the chance of that happening I'm making the parameter checking as strict as possible, and ensuring that a string (of the correct length) isn't passed in by accident. As you say, the chances of that happening are tiny, but the results are severe (a core dump). It appears to be working correctly in the latest version.
I've also rebuilt the plugin on Windows 32bit and had to make a few more changes:
- The combination of #cppIf:ifTrue:[ifFalse:] and more agressive
inlining means that smalltalk #ifTrue:#ifFalse: message sends are translated to C ternary conditional statements (?:). If the smalltalk #ifTrue:#ifFalse: contains a return (^), this can end up generating invalid C code, sometimes trying to return from within the ternary conditional, and sometimes trying to goto out of the conditional.
- There was a bug in the time zone handling on Windows, which I've fixed.
The latest version is now:
Name: FileAttributesPlugin.oscog-AlistairGrant.26 Author: AlistairGrant Time: 5 January 2018, 9:02:46.033079 am UUID: 07c4e352-8193-452a-a80a-440f0811d43f Ancestors: FileAttributesPlugin.oscog-AlistairGrant.25
Version 1.2.4:
- Adjust for #cppIf:ifTrue:[ifFalse:] and inlining behaviour
-- Smalltalk #ifTrue:#ifFalse: messages appear to be translated to C ternary operator instead of a if[else] statement.
- Remove call to sqFileInit() (accidentally copied across from FilePlugin).
- Fix stat() time zone adjustments on WIN32
Thanks! Alistair
Updated version:
Name: FileAttributesPlugin.oscog-AlistairGrant.25 Author: AlistairGrant Time: 4 January 2018, 4:02:11.405148 pm UUID: 5b76e8bb-69e1-4db4-b529-9de1249e63e0 Ancestors: FileAttributesPlugin.oscog-eem.24
Version 1.2.3:
- Ensure that #pointerFrom: is given a ByteArray, not just a byte
object, which can be a string.
Thanks, Alistair
On 4 January 2018 at 00:05, Alistair Grant akgrant0710@gmail.com wrote: Hi Eliot,
I'm able to reliably crash the 64 bit VM by passing a string to primitiveClosedir in the FileAttributesPlugin (which should never be done in practice, I've got it as a test for the parameter checking):
primitiveClosedir "Close the directory stream for dirPointerOop. Answer dirPointerOop on success. Raise PrimErrBadArgument if the parameter is not a ByteArray length size(void *). If closedir() returns an error raise PrimitiveOSError."
| dirPointerOop dirStream result | <export: true> <var: 'dirStream' type: 'osdir *'> dirPointerOop := interpreterProxy stackValue: 0.
">>>> Putting the following back prevents the crash..." (interpreterProxy is: dirPointerOop KindOf: 'ByteArray') ifFalse: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. "<<<<"
dirStream := self pointerFrom: dirPointerOop. dirStream ifNil: [^interpreterProxy primitiveFailFor: PrimErrBadArgument]. result := self closedir: dirStream dp. result = 0 ifFalse: [^interpreterProxy primitiveFailForOSError: self unableToCloseDir]. self free: dirStream. interpreterProxy pop: 2 thenPush: dirPointerOop
I removed the explicit is:KindOf: ByteArray check as you said that the checks in pointerFrom: would be sufficient.
Would you mind letting me know what your preferred solution is (put the explicit ByteArray check back in, or look to fix the checks in pointerFrom:, or ...)?
Just for reference, pointerFrom: is:
pointerFrom: directoryPointerBytes "Answer the machine address contained in anExternalAddressOop."
| ptr addressUnion idx | <returnTypeC: 'void *'> <var: 'ptr' type: 'unsigned char *'> <var: 'addressUnion' type: 'union {void *address; unsigned char bytes[sizeof(void *)];}'> ((interpreterProxy isBytes: directoryPointerBytes) and: [(interpreterProxy stSizeOf: directoryPointerBytes) = self sizeOfPointer]) ifFalse: [^ nil]. ptr := interpreterProxy arrayValueOf: directoryPointerBytes. idx := 0. [idx < self sizeOfPointer] whileTrue: [self cCode: 'addressUnion.bytes[idx] = ptr[idx]'. idx := idx + 1]. ^ self cCode: 'addressUnion.address' inSmalltalk: [addressUnion]
Thanks, Alistair
On 3 January 2018 at 20:45, Alistair Grant akgrant0710@gmail.com wrote: Hi Eliot,
On 3 January 2018 at 20:20, Alistair Grant akgrant0710@gmail.com wrote: Hi Eliot,
> On 3 January 2018 at 19:04, Eliot Miranda eliot.miranda@gmail.com wrote: > > Hi Alistair, > >> On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant akgrant0710@gmail.com wrote: >> >> >> Hi Jakob, >> >>> On 2 January 2018 at 06:46, Jakob Reschke forums.jakob@resfarm.de wrote: >>> >>> >>> Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com: >>> >>> ... 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.
OK.
> 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.
Yes, please :-)
My build system has suddenly decided that it can't find libpulse-simple. I can't see that any of the changes you've made will impact it, but I'll work on getting my build working again and running it through the test suite.
Deleting the running docker container and going back to the clean image fixed the libpulse-simple problem.
And all the automated tests pass (FileAttributesPlugin.oscog-eem.22, Ubuntu 16.04).
Thanks! Alistair
On 01-01-2018, at 9:46 PM, Jakob Reschke forums.jakob@resfarm.de wrote:
Am 02.01.2018 04:43 schrieb "Eliot Miranda" eliot.miranda@gmail.com: ... 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; tim@rowledge.org; 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!"
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/FileAttri...
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 akgrant0710@gmail.com 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.19Looking 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
Hi Clément,
On Dec 22, 2017, at 2:29 AM, Clément Bera bera.clement@gmail.com wrote:
Hi,
What is the status of the FilesAttributesPlugin ? I would like to make it available by default on pharo VMs.
Questions:
Is it already merged with another plugin ?
If not, for integration, should I move the FilesAttributesPlugin from its external smalltalkhub repository to VMMaker-Plugins package ?
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 https://clementbera.wordpress.com/ Bâtiment B 40, avenue Halley 59650 Villeneuve d'Ascq
vm-dev@lists.squeakfoundation.org
-
Alistair Grant -
Clément Bera -
Eliot Miranda -
Jakob Reschke -
tim Rowledge