[Vm-dev] [OpenSmalltalk/opensmalltalk-vm] FileAttributesPlugin 2.0.6 (#321)

[Eliot Miranda]

Hi Alistair,

   you can do better than using malloc & free in primitiveClosedir et al.
If you look at the changes I made in VMMaker.oscog-eem.2490
& VMMaker.oscog-eem.2491 you'll see a simple pattern:

SerialPlugin>>copyPortNameToCString: portName
<returnTypeC: #'char *'>
| port portNameSize |
<inline: #always>
<var: 'port' type: #'char *'>
portNameSize := interpreterProxy slotSizeOf: (portName asOop: String).
port := self alloca: portNameSize + 1.
self memcpy: port _: portName _: portNameSize.
port at: portNameSize put: 0.
^port

This method simply copies a Smalltalk string to a null-terminated C string
via alloca.  If you don't know this, alloca is like malloc but it allocates
on the stack in the current stack frame and so the space it allocates is
automatically freed on return from the function in which alloca is
invoked.  Because the method is marked <inline: #always> it is in lined
into its caller and so the alloca happens in the calling method, e.g.

SerialPlugin>>primitiveSerialPortCloseByName: portName
| port |
self primitive: 'primitiveSerialPortCloseByName'
parameters: #(String).
port := self copyPortNameToCString: portName.
self serialPortCloseByName: port

and there is no need to free anything.

Hence in e.g. primitiveClosedir where you write

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 faPathPtr faPath result |
<export: true>
<var: 'faPath' type: #'fapath *'>
<var: 'faPathPtr' type: #'fapathptr *'>

dirPointerOop := interpreterProxy stackValue: 0.
faPathPtr := self cCode: '(fapathptr *)structFromObjectsize(dirPointerOop,
sizeof(fapathptr))'
inSmalltalk: [self structFromObject: dirPointerOop size: self
sizeOfFaPathPtr].
faPathPtr = 0 ifTrue:
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].
(self faValidateSessionId: (self cCode: 'faPathPtr->sessionId' inSmalltalk:
[faPathPtr first])) ifFalse:
[self free: faPathPtr.
^interpreterProxy primitiveFailFor: PrimErrBadArgument].
faPath := self cCode: 'faPathPtr->faPath' inSmalltalk: [faPathPtr second].

result := self faCloseDirectory: faPath.
self faInvalidateSessionId: (self cCode: '&faPathPtr->sessionId'
inSmalltalk: [faPathPtr]).
result = 0 ifFalse:
[^interpreterProxy primitiveFailForOSError: result].
self free: faPathPtr.
self free: faPath.
interpreterProxy pop: 2 thenPush: dirPointerOop

if you implement structFromObject:size: like this:

FileAttributesPlugin>>structFromObject: anObject size: structSize
"Allocate memory of the requiested size and copy the contents of anObject
in to it.
anObject is expected to be bytes, e.g. ByteArray or String.
Use alloca to avoid having to explicitly free memory."

<returnTypeC: #'void *'>
| buffer |
<inline: #always>

(interpreterProxy stSizeOf: anObject) = structSize ifFalse:
[interpreterProxy primitiveFailFor: PrimErrBadArgument.
^0].
buffer := self alloca: structSize.
buffer = 0
ifTrue: [interpreterProxy primitiveFailFor: PrimErrNoCMemory]
ifFalse:
[self memcpy: buffer
_: (interpreterProxy arrayValueOf: anObject)
_: structSize].
^buffer

you can then write primitiveCloseDir as

FileAttributesPlugin>>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 faPathPtr faPath result |
<export: true>
<var: 'faPath' type: #'fapath *'>
<var: 'faPathPtr' type: #'fapathptr *'>

dirPointerOop := interpreterProxy stackValue: 0.
faPathPtr := self structFromObject: dirPointerOop size: (self sizeof:
faPathPtr)].
faPathPtr = 0 ifTrue:
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].
(self faValidateSessionId: (self cCode: 'faPathPtr->sessionId' inSmalltalk:
[faPathPtr first])) ifFalse:
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].
faPath := self cCode: 'faPathPtr->faPath' inSmalltalk: [faPathPtr second].

result := self faCloseDirectory: faPath.
self faInvalidateSessionId: (self cCode: '&faPathPtr->sessionId'
inSmalltalk: [faPathPtr]).
result = 0 ifFalse:
[^interpreterProxy primitiveFailForOSError: result].
self free: faPath.
interpreterProxy pop: 2 thenPush: dirPointerOop

If you would go as far as to make a VMStructType subclass for fapath et al
you could then write it as

FileAttributesPlugin>>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 faPathPtr result |
<export: true>
<var: 'faPathPtr' type: #'FAPathptr *'>

dirPointerOop := interpreterProxy stackValue: 0.
faPathPtr := self structFromObject: dirPointerOop size: (self sizeof:
faPathPtr)].
faPathPtr = 0 ifTrue:
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].
(self faValidateSessionId: faPathPtr sessionId) ifFalse:
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].

result := self faCloseDirectory: faPathPtr faPath.
self faInvalidateSessionId: faPathPtr sessionId.
result = 0 ifFalse:
[^interpreterProxy primitiveFailForOSError: result].
self free: faPathPtr faPath.
interpreterProxy pop: 2 thenPush: dirPointerOop

and then as

FileAttributesPlugin>>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 faPathPtr result |
<export: true>
<var: 'faPathPtr' type: #'FAPathptr *'>

dirPointerOop := interpreterProxy stackValue: 0.
faPathPtr := self structFromObject: dirPointerOop size: (self sizeof:
faPathPtr)].
(faPathPtr ~= 0
and: [self faValidateSessionId: faPathPtr sessionId]) ifFalse:
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].

result := self faCloseDirectory: faPathPtr faPath.
self faInvalidateSessionId: faPathPtr sessionId.
result = 0 ifFalse:
[^interpreterProxy primitiveFailForOSError: result].
self free: faPathPtr faPath.
interpreterProxy pop: 2 thenPush: dirPointerOop

The only thing that;'s unclear for me at the moment is how the result of
faInvalidateSessionId: gets tested.  result doesn't get set by it.  I
presume it sets primitive failure, but there';s no test for primitive
failure where result is tested (result = 0).

HTH!

On Sat, Dec 15, 2018 at 1:10 AM akgrant43 <notifications at github.com> wrote:

> Merged #321 <https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321>
> into Cog.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321#event-2028387352>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/APHa0Kry-Zrt5kUswbgFCps_HW63I6gEks5u5LxogaJpZM4ZT4Mm>
> .
>


-- 
_,,,^..^,,,_
best, Eliot


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321#issuecomment-447607424
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20181215/e10fd221/attachment.html>