<div dir="ltr">Hi Alistair,<div class="gmail_extra"><br><div class="gmail_quote">On Thu, Mar 1, 2018 at 11:21 AM, Alistair Grant <span dir="ltr"><<a href="mailto:akgrant0710@gmail.com" target="_blank">akgrant0710@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><br>
Hi Everyone,<br>
<br>
I'm currently extending FilePlugin to allow files to be opened using<br>
either a file descriptor or FILE*. If you're interested more details<br>
at:<br>
<br>
Original PR: <a href="https://github.com/pharo-project/pharo-vm/pull/108" rel="noreferrer" target="_blank">https://github.com/pharo-<wbr>project/pharo-vm/pull/108</a><br>
Updated PR: <a href="https://github.com/pharo-project/pharo-vm/pull/142" rel="noreferrer" target="_blank">https://github.com/pharo-<wbr>project/pharo-vm/pull/142</a><br>
<br>
<br>
While tidying up the argument checking I noticed that FilePlugin has as<br>
part of #primitiveFileOpen:<br>
<br>
<br>
namePointer := interpreterProxy stackValue: 1.<br>
(interpreterProxy isBytes: namePointer)<br>
ifFalse: [^ interpreterProxy primitiveFail].<br>
<br>
<br>
My understanding is that this is bad practice as #primitiveFail won't<br>
correctly retry if the argument is a forwarder. </blockquote><div><br></div><div>No, that's not so. Any primitive failure is fine. Remember that when we discussed this the FileAttributesPlugin was answering error codes as the result of a primitive instead of failing. This is the approach that won't work. What happens is that when a primitive fails (with any code including nil, the non-code used when failing via primitiveFail) is that the VM scans the parameters to the primitive, following forwarders, and retries the primitive if it finds any. So there's no need to change primitiveFail into primitiveFailFor: PrimErrXXX.</div><div><br></div><div>However, good error codes can allow much nicer primitive failure code. Testing the error code can be faster and simpler than interrogating the arguments to the primitive. For example, here are two versions of ByteString>>at:put: that deal with read-only objects.</div><div><br></div><div><div>at: index put: aCharacter</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>"Primitive. Store the Character in the field of the receiver indicated by</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>the index. Fail if the index is not an Integer or is out of bounds, or if</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>the argument is not a Character. Essential. See Object documentation</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>whatIsAPrimitive."</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><primitive: 64 error: ec></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>aCharacter isCharacter </div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>ifFalse:[^self errorImproperStore].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>aCharacter isOctetCharacter ifFalse:[</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>"Convert to WideString"</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>self becomeForward: (WideString from: self).</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>^self at: index put: aCharacter.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>index isInteger</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>ifTrue: [ (index between: 1 and: self size)</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>ifFalse: [ self errorSubscriptBounds: index ] ]</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>ifFalse: [self errorNonIntegerIndex].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><b><i>self isReadOnlyObject </i></b></div><div><b><i><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>ifTrue: [ ^ self modificationForbiddenFor: #at:put: index: index value: aCharacter ]</i></b></div></div><div><br></div><div>and now testing the error code directly:</div><div><br></div><div><div>at: index put: aCharacter</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>"Primitive. Store the Character in the field of the receiver indicated by the index.</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span> Fail if the index is not an Integer or is out of bounds, or if the argument is not a</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span> Character, or the Character's code is outside the 0-255 range, or if the receiver</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span> is read-only. Essential. See Object documentation whatIsAPrimitive."</div><div><br></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span><primitive: 64 error: ec></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>aCharacter isCharacter ifFalse:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>[^self errorImproperStore].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>index isInteger</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>ifTrue:</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>[<b><i>ec == #'no modification' ifTrue:</i></b></div><div><b><i><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>[^self modificationForbiddenFor: #at:put: index: index value: aCharacter].</i></b></div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span> aCharacter isOctetCharacter ifFalse: "Convert to WideString"</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>[self becomeForward: (WideString from: self).</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>^self at: index put: aCharacter].</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span> self errorSubscriptBounds: index]</div><div><span class="gmail-Apple-tab-span" style="white-space:pre"> </span>ifFalse: [self errorNonIntegerIndex]</div></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> So it should be:<br>
<br>
<br>
namePointer := interpreterProxy stackValue: 1.<br>
(interpreterProxy isBytes: namePointer)<br>
ifFalse: [^ interpreterProxy primitiveFailFor: PrimErrBadArgument].<br>
<br>
<br>
Can someone confirm that I'm not misunderstanding, and I'll go through<br>
and check FilePlugin further.<br></blockquote><div><br></div><div>Feel free to add better error codes, but do understand that it is orthogonal to the forwarder following issue. The important thing is that primitives must fail, not answer error codes as results, i.e. that primitives be implemented as they always have been.</div><div></div></div><div class="gmail_extra"><br></div>Cheers!</div><div class="gmail_extra"><br><div class="gmail_signature"><div dir="ltr"><div><span style="font-size:small;border-collapse:separate"><div>_,,,^..^,,,_<br></div><div>best, Eliot</div></span></div></div></div>
</div></div>