Hi Alistair,<br>
<br>
   you can do better than using malloc & free in primitiveClosedir et al.<br>
If you look at the changes I made in VMMaker.oscog-eem.2490<br>
& VMMaker.oscog-eem.2491 you'll see a simple pattern:<br>
<br>
SerialPlugin>>copyPortNameToCString: portName<br>
<returnTypeC: #'char *'><br>
| port portNameSize |<br>
<inline: #always><br>
<var: 'port' type: #'char *'><br>
portNameSize := interpreterProxy slotSizeOf: (portName asOop: String).<br>
port := self alloca: portNameSize + 1.<br>
self memcpy: port _: portName _: portNameSize.<br>
port at: portNameSize put: 0.<br>
^port<br>
<br>
This method simply copies a Smalltalk string to a null-terminated C string<br>
via alloca.  If you don't know this, alloca is like malloc but it allocates<br>
on the stack in the current stack frame and so the space it allocates is<br>
automatically freed on return from the function in which alloca is<br>
invoked.  Because the method is marked <inline: #always> it is in lined<br>
into its caller and so the alloca happens in the calling method, e.g.<br>
<br>
SerialPlugin>>primitiveSerialPortCloseByName: portName<br>
| port |<br>
self primitive: 'primitiveSerialPortCloseByName'<br>
parameters: #(String).<br>
port := self copyPortNameToCString: portName.<br>
self serialPortCloseByName: port<br>
<br>
and there is no need to free anything.<br>
<br>
Hence in e.g. primitiveClosedir where you write<br>
<br>
primitiveClosedir<br>
"Close the directory stream for dirPointerOop. Answer dirPointerOop on<br>
success.<br>
Raise PrimErrBadArgument if the parameter is not a ByteArray length<br>
size(void *).<br>
If closedir() returns an error raise PrimitiveOSError."<br>
<br>
| dirPointerOop faPathPtr faPath result |<br>
<export: true><br>
<var: 'faPath' type: #'fapath *'><br>
<var: 'faPathPtr' type: #'fapathptr *'><br>
<br>
dirPointerOop := interpreterProxy stackValue: 0.<br>
faPathPtr := self cCode: '(fapathptr *)structFromObjectsize(dirPointerOop,<br>
sizeof(fapathptr))'<br>
inSmalltalk: [self structFromObject: dirPointerOop size: self<br>
sizeOfFaPathPtr].<br>
faPathPtr = 0 ifTrue:<br>
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].<br>
(self faValidateSessionId: (self cCode: 'faPathPtr->sessionId' inSmalltalk:<br>
[faPathPtr first])) ifFalse:<br>
[self free: faPathPtr.<br>
^interpreterProxy primitiveFailFor: PrimErrBadArgument].<br>
faPath := self cCode: 'faPathPtr->faPath' inSmalltalk: [faPathPtr second].<br>
<br>
result := self faCloseDirectory: faPath.<br>
self faInvalidateSessionId: (self cCode: '&faPathPtr->sessionId'<br>
inSmalltalk: [faPathPtr]).<br>
result = 0 ifFalse:<br>
[^interpreterProxy primitiveFailForOSError: result].<br>
self free: faPathPtr.<br>
self free: faPath.<br>
interpreterProxy pop: 2 thenPush: dirPointerOop<br>
<br>
if you implement structFromObject:size: like this:<br>
<br>
FileAttributesPlugin>>structFromObject: anObject size: structSize<br>
"Allocate memory of the requiested size and copy the contents of anObject<br>
in to it.<br>
anObject is expected to be bytes, e.g. ByteArray or String.<br>
Use alloca to avoid having to explicitly free memory."<br>
<br>
<returnTypeC: #'void *'><br>
| buffer |<br>
<inline: #always><br>
<br>
(interpreterProxy stSizeOf: anObject) = structSize ifFalse:<br>
[interpreterProxy primitiveFailFor: PrimErrBadArgument.<br>
^0].<br>
buffer := self alloca: structSize.<br>
buffer = 0<br>
ifTrue: [interpreterProxy primitiveFailFor: PrimErrNoCMemory]<br>
ifFalse:<br>
[self memcpy: buffer<br>
_: (interpreterProxy arrayValueOf: anObject)<br>
_: structSize].<br>
^buffer<br>
<br>
you can then write primitiveCloseDir as<br>
<br>
FileAttributesPlugin>>primitiveClosedir<br>
"Close the directory stream for dirPointerOop. Answer dirPointerOop on<br>
success.<br>
Raise PrimErrBadArgument if the parameter is not a ByteArray length<br>
size(void *).<br>
If closedir() returns an error raise PrimitiveOSError."<br>
<br>
| dirPointerOop faPathPtr faPath result |<br>
<export: true><br>
<var: 'faPath' type: #'fapath *'><br>
<var: 'faPathPtr' type: #'fapathptr *'><br>
<br>
dirPointerOop := interpreterProxy stackValue: 0.<br>
faPathPtr := self structFromObject: dirPointerOop size: (self sizeof:<br>
faPathPtr)].<br>
faPathPtr = 0 ifTrue:<br>
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].<br>
(self faValidateSessionId: (self cCode: 'faPathPtr->sessionId' inSmalltalk:<br>
[faPathPtr first])) ifFalse:<br>
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].<br>
faPath := self cCode: 'faPathPtr->faPath' inSmalltalk: [faPathPtr second].<br>
<br>
result := self faCloseDirectory: faPath.<br>
self faInvalidateSessionId: (self cCode: '&faPathPtr->sessionId'<br>
inSmalltalk: [faPathPtr]).<br>
result = 0 ifFalse:<br>
[^interpreterProxy primitiveFailForOSError: result].<br>
self free: faPath.<br>
interpreterProxy pop: 2 thenPush: dirPointerOop<br>
<br>
If you would go as far as to make a VMStructType subclass for fapath et al<br>
you could then write it as<br>
<br>
FileAttributesPlugin>>primitiveClosedir<br>
"Close the directory stream for dirPointerOop. Answer dirPointerOop on<br>
success.<br>
Raise PrimErrBadArgument if the parameter is not a ByteArray length<br>
size(void *).<br>
If closedir() returns an error raise PrimitiveOSError."<br>
<br>
| dirPointerOop faPathPtr result |<br>
<export: true><br>
<var: 'faPathPtr' type: #'FAPathptr *'><br>
<br>
dirPointerOop := interpreterProxy stackValue: 0.<br>
faPathPtr := self structFromObject: dirPointerOop size: (self sizeof:<br>
faPathPtr)].<br>
faPathPtr = 0 ifTrue:<br>
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].<br>
(self faValidateSessionId: faPathPtr sessionId) ifFalse:<br>
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].<br>
<br>
result := self faCloseDirectory: faPathPtr faPath.<br>
self faInvalidateSessionId: faPathPtr sessionId.<br>
result = 0 ifFalse:<br>
[^interpreterProxy primitiveFailForOSError: result].<br>
self free: faPathPtr faPath.<br>
interpreterProxy pop: 2 thenPush: dirPointerOop<br>
<br>
and then as<br>
<br>
FileAttributesPlugin>>primitiveClosedir<br>
"Close the directory stream for dirPointerOop. Answer dirPointerOop on<br>
success.<br>
Raise PrimErrBadArgument if the parameter is not a ByteArray length<br>
size(void *).<br>
If closedir() returns an error raise PrimitiveOSError."<br>
<br>
| dirPointerOop faPathPtr result |<br>
<export: true><br>
<var: 'faPathPtr' type: #'FAPathptr *'><br>
<br>
dirPointerOop := interpreterProxy stackValue: 0.<br>
faPathPtr := self structFromObject: dirPointerOop size: (self sizeof:<br>
faPathPtr)].<br>
(faPathPtr ~= 0<br>
and: [self faValidateSessionId: faPathPtr sessionId]) ifFalse:<br>
[^interpreterProxy primitiveFailFor: PrimErrBadArgument].<br>
<br>
result := self faCloseDirectory: faPathPtr faPath.<br>
self faInvalidateSessionId: faPathPtr sessionId.<br>
result = 0 ifFalse:<br>
[^interpreterProxy primitiveFailForOSError: result].<br>
self free: faPathPtr faPath.<br>
interpreterProxy pop: 2 thenPush: dirPointerOop<br>
<br>
The only thing that;'s unclear for me at the moment is how the result of<br>
faInvalidateSessionId: gets tested.  result doesn't get set by it.  I<br>
presume it sets primitive failure, but there';s no test for primitive<br>
failure where result is tested (result = 0).<br>
<br>
HTH!<br>
<br>
On Sat, Dec 15, 2018 at 1:10 AM akgrant43 <notifications@github.com> wrote:<br>
<br>
> Merged #321 <https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321><br>
> into Cog.<br>
><br>
> —<br>
> You are receiving this because you are subscribed to this thread.<br>
> Reply to this email directly, view it on GitHub<br>
> <https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321#event-2028387352>,<br>
> or mute the thread<br>
> <https://github.com/notifications/unsubscribe-auth/APHa0Kry-Zrt5kUswbgFCps_HW63I6gEks5u5LxogaJpZM4ZT4Mm><br>
> .<br>
><br>
<br>
<br>
-- <br>
_,,,^..^,,,_<br>
best, Eliot<br>


<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321#issuecomment-447607424">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AhLyW8rPPfiSPtyUZPMN4CQt4W_f1PXZks5u5Y-1gaJpZM4ZT4Mm">mute the thread</a>.<img src="https://github.com/notifications/beacon/AhLyW5c4UTa423xamllJmXTH8kopxq5Jks5u5Y-1gaJpZM4ZT4Mm.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/OpenSmalltalk/opensmalltalk-vm","title":"OpenSmalltalk/opensmalltalk-vm","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm"}},"updates":{"snippets":[{"icon":"PERSON","message":"@eliotmiranda in #321: Hi Alistair,\n\n   you can do better than using malloc \u0026 free in primitiveClosedir et al.\nIf you look at the changes I made in VMMaker.oscog-eem.2490\n\u0026 VMMaker.oscog-eem.2491 you'll see a simple pattern:\n\nSerialPlugin\u003e\u003ecopyPortNameToCString: portName\n\u003creturnTypeC: #'char *'\u003e\n| port portNameSize |\n\u003cinline: #always\u003e\n\u003cvar: 'port' type: #'char *'\u003e\nportNameSize := interpreterProxy slotSizeOf: (portName asOop: String).\nport := self alloca: portNameSize + 1.\nself memcpy: port _: portName _: portNameSize.\nport at: portNameSize put: 0.\n^port\n\nThis method simply copies a Smalltalk string to a null-terminated C string\nvia alloca.  If you don't know this, alloca is like malloc but it allocates\non the stack in the current stack frame and so the space it allocates is\nautomatically freed on return from the function in which alloca is\ninvoked.  Because the method is marked \u003cinline: #always\u003e it is in lined\ninto its caller and so the alloca happens in the calling method, e.g.\n\nSerialPlugin\u003e\u003eprimitiveSerialPortCloseByName: portName\n| port |\nself primitive: 'primitiveSerialPortCloseByName'\nparameters: #(String).\nport := self copyPortNameToCString: portName.\nself serialPortCloseByName: port\n\nand there is no need to free anything.\n\nHence in e.g. primitiveClosedir where you write\n\nprimitiveClosedir\n\"Close the directory stream for dirPointerOop. Answer dirPointerOop on\nsuccess.\nRaise PrimErrBadArgument if the parameter is not a ByteArray length\nsize(void *).\nIf closedir() returns an error raise PrimitiveOSError.\"\n\n| dirPointerOop faPathPtr faPath result |\n\u003cexport: true\u003e\n\u003cvar: 'faPath' type: #'fapath *'\u003e\n\u003cvar: 'faPathPtr' type: #'fapathptr *'\u003e\n\ndirPointerOop := interpreterProxy stackValue: 0.\nfaPathPtr := self cCode: '(fapathptr *)structFromObjectsize(dirPointerOop,\nsizeof(fapathptr))'\ninSmalltalk: [self structFromObject: dirPointerOop size: self\nsizeOfFaPathPtr].\nfaPathPtr = 0 ifTrue:\n[^interpreterProxy primitiveFailFor: PrimErrBadArgument].\n(self faValidateSessionId: (self cCode: 'faPathPtr-\u003esessionId' inSmalltalk:\n[faPathPtr first])) ifFalse:\n[self free: faPathPtr.\n^interpreterProxy primitiveFailFor: PrimErrBadArgument].\nfaPath := self cCode: 'faPathPtr-\u003efaPath' inSmalltalk: [faPathPtr second].\n\nresult := self faCloseDirectory: faPath.\nself faInvalidateSessionId: (self cCode: '\u0026faPathPtr-\u003esessionId'\ninSmalltalk: [faPathPtr]).\nresult = 0 ifFalse:\n[^interpreterProxy primitiveFailForOSError: result].\nself free: faPathPtr.\nself free: faPath.\ninterpreterProxy pop: 2 thenPush: dirPointerOop\n\nif you implement structFromObject:size: like this:\n\nFileAttributesPlugin\u003e\u003estructFromObject: anObject size: structSize\n\"Allocate memory of the requiested size and copy the contents of anObject\nin to it.\nanObject is expected to be bytes, e.g. ByteArray or String.\nUse alloca to avoid having to explicitly free memory.\"\n\n\u003creturnTypeC: #'void *'\u003e\n| buffer |\n\u003cinline: #always\u003e\n\n(interpreterProxy stSizeOf: anObject) = structSize ifFalse:\n[interpreterProxy primitiveFailFor: PrimErrBadArgument.\n^0].\nbuffer := self alloca: structSize.\nbuffer = 0\nifTrue: [interpreterProxy primitiveFailFor: PrimErrNoCMemory]\nifFalse:\n[self memcpy: buffer\n_: (interpreterProxy arrayValueOf: anObject)\n_: structSize].\n^buffer\n\nyou can then write primitiveCloseDir as\n\nFileAttributesPlugin\u003e\u003eprimitiveClosedir\n\"Close the directory stream for dirPointerOop. Answer dirPointerOop on\nsuccess.\nRaise PrimErrBadArgument if the parameter is not a ByteArray length\nsize(void *).\nIf closedir() returns an error raise PrimitiveOSError.\"\n\n| dirPointerOop faPathPtr faPath result |\n\u003cexport: true\u003e\n\u003cvar: 'faPath' type: #'fapath *'\u003e\n\u003cvar: 'faPathPtr' type: #'fapathptr *'\u003e\n\ndirPointerOop := interpreterProxy stackValue: 0.\nfaPathPtr := self structFromObject: dirPointerOop size: (self sizeof:\nfaPathPtr)].\nfaPathPtr = 0 ifTrue:\n[^interpreterProxy primitiveFailFor: PrimErrBadArgument].\n(self faValidateSessionId: (self cCode: 'faPathPtr-\u003esessionId' inSmalltalk:\n[faPathPtr first])) ifFalse:\n[^interpreterProxy primitiveFailFor: PrimErrBadArgument].\nfaPath := self cCode: 'faPathPtr-\u003efaPath' inSmalltalk: [faPathPtr second].\n\nresult := self faCloseDirectory: faPath.\nself faInvalidateSessionId: (self cCode: '\u0026faPathPtr-\u003esessionId'\ninSmalltalk: [faPathPtr]).\nresult = 0 ifFalse:\n[^interpreterProxy primitiveFailForOSError: result].\nself free: faPath.\ninterpreterProxy pop: 2 thenPush: dirPointerOop\n\nIf you would go as far as to make a VMStructType subclass for fapath et al\nyou could then write it as\n\nFileAttributesPlugin\u003e\u003eprimitiveClosedir\n\"Close the directory stream for dirPointerOop. Answer dirPointerOop on\nsuccess.\nRaise PrimErrBadArgument if the parameter is not a ByteArray length\nsize(void *).\nIf closedir() returns an error raise PrimitiveOSError.\"\n\n| dirPointerOop faPathPtr result |\n\u003cexport: true\u003e\n\u003cvar: 'faPathPtr' type: #'FAPathptr *'\u003e\n\ndirPointerOop := interpreterProxy stackValue: 0.\nfaPathPtr := self structFromObject: dirPointerOop size: (self sizeof:\nfaPathPtr)].\nfaPathPtr = 0 ifTrue:\n[^interpreterProxy primitiveFailFor: PrimErrBadArgument].\n(self faValidateSessionId: faPathPtr sessionId) ifFalse:\n[^interpreterProxy primitiveFailFor: PrimErrBadArgument].\n\nresult := self faCloseDirectory: faPathPtr faPath.\nself faInvalidateSessionId: faPathPtr sessionId.\nresult = 0 ifFalse:\n[^interpreterProxy primitiveFailForOSError: result].\nself free: faPathPtr faPath.\ninterpreterProxy pop: 2 thenPush: dirPointerOop\n\nand then as\n\nFileAttributesPlugin\u003e\u003eprimitiveClosedir\n\"Close the directory stream for dirPointerOop. Answer dirPointerOop on\nsuccess.\nRaise PrimErrBadArgument if the parameter is not a ByteArray length\nsize(void *).\nIf closedir() returns an error raise PrimitiveOSError.\"\n\n| dirPointerOop faPathPtr result |\n\u003cexport: true\u003e\n\u003cvar: 'faPathPtr' type: #'FAPathptr *'\u003e\n\ndirPointerOop := interpreterProxy stackValue: 0.\nfaPathPtr := self structFromObject: dirPointerOop size: (self sizeof:\nfaPathPtr)].\n(faPathPtr ~= 0\nand: [self faValidateSessionId: faPathPtr sessionId]) ifFalse:\n[^interpreterProxy primitiveFailFor: PrimErrBadArgument].\n\nresult := self faCloseDirectory: faPathPtr faPath.\nself faInvalidateSessionId: faPathPtr sessionId.\nresult = 0 ifFalse:\n[^interpreterProxy primitiveFailForOSError: result].\nself free: faPathPtr faPath.\ninterpreterProxy pop: 2 thenPush: dirPointerOop\n\nThe only thing that;'s unclear for me at the moment is how the result of\nfaInvalidateSessionId: gets tested.  result doesn't get set by it.  I\npresume it sets primitive failure, but there';s no test for primitive\nfailure where result is tested (result = 0).\n\nHTH!\n\nOn Sat, Dec 15, 2018 at 1:10 AM akgrant43 \u003cnotifications@github.com\u003e wrote:\n\n\u003e Merged #321 \u003chttps://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321\u003e\n\u003e into Cog.\n\u003e\n\u003e —\n\u003e You are receiving this because you are subscribed to this thread.\n\u003e Reply to this email directly, view it on GitHub\n\u003e \u003chttps://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321#event-2028387352\u003e,\n\u003e or mute the thread\n\u003e \u003chttps://github.com/notifications/unsubscribe-auth/APHa0Kry-Zrt5kUswbgFCps_HW63I6gEks5u5LxogaJpZM4ZT4Mm\u003e\n\u003e .\n\u003e\n\n\n-- \n_,,,^..^,,,_\nbest, Eliot\n"}],"action":{"name":"View Pull Request","url":"https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321#issuecomment-447607424"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321#issuecomment-447607424",
"url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/pull/321#issuecomment-447607424",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>