[Vm-dev] VM Maker: VMMaker.oscog-eem.2347.mcz

[Eliot Miranda]

Hi Alistair,

> On Mar 8, 2018, at 7:12 AM, Alistair Grant <akgrant0710 at gmail.com> wrote:
> 
> 
> Hi Levente,
> 
> 
>> On 8 March 2018 at 00:44, Levente Uzonyi <leves at caesar.elte.hu> wrote:
>> 
>>> On Wed, 7 Mar 2018, commits at source.squeak.org wrote:
>>> 
>>> 
>>> Eliot Miranda uploaded a new version of VMMaker to project VM Maker:
>>> http://source.squeak.org/VMMaker/VMMaker.oscog-eem.2347.mcz
>>> 
>> 
>> snip
>> 
>>> Item was changed:
>>> ----- Method: FilePlugin>>primitiveConnectToFile (in category 'file
>>> primitives') -----
>>> primitiveConnectToFile
>>>        "Connect to the file with the supplied FILE* and writeFlag.
>>>        FILE* must be supplied in a byte object (ByteArray) with the
>>> platform address size.
>>>        writeFlag must be a boolean and compatible with the existing file
>>> access."
>>>        | writeFlag cfileOop cfile filePointer |
>>> +       <var: 'cfile' type: #'void *'>
>>> -       <var: 'cfile' type: 'void* '>
>>>        <export: true>
>>> +       writeFlag := interpreterProxy booleanValueOf: (interpreterProxy
>>> stackValue: 0).
>>> -       writeFlag := interpreterProxy
>>> -                               booleanValueOf: (interpreterProxy
>>> stackValue: 0).
>> 
>> 
>> It should be checked here if the primitive has failed or not, shouldn't it?
> 
> Yes.  I thought I'd taken care of this, but obviously not.
> 
> 
>>>        cfileOop := interpreterProxy stackValue: 1.
>>>        cfile := self pointerFrom: cfileOop.
>>>        cfile ifNil:
>>>                [^interpreterProxy primitiveFailFor: PrimErrBadArgument].
>>>        filePointer := self connectToFile: cfile write: writeFlag.
>>>        interpreterProxy failed ifFalse: +              [interpreterProxy
>>> pop: 3 "rcvr, name, writeFlag"
>>> +                                                       thenPush:
>>> filePointer]!
>>> -               [^interpreterProxy pop: 3 "rcvr, name, writeFlag"
>>> -                                                       thenPush:
>>> filePointer].
>>> -       ^interpreterProxy primitiveFail.!
>>> 
>>> Item was changed:
>>> ----- Method: FilePlugin>>primitiveConnectToFileDescriptor (in category
>>> 'file primitives') -----
>>> primitiveConnectToFileDescriptor
>>>        "Connect to the existing file identified by fileDescriptor.
>>>        fileDescriptor must be an integer.
>>>        writeFlag is aboolean indicating whether to open in read or write
>>> mode and must be compatible with the existing file access."
>>>        | writeFlag fdPointer fd filePointer |
>>> +       <var: 'fd' type: #int>
>>> -       <var: 'fd' type: 'int'>
>>>        <export: true>
>>>        writeFlag := interpreterProxy
>>>                                booleanValueOf: (interpreterProxy
>>> stackValue: 0).
>> 
>> 
>> Same as above.
> 
> I'll change both of these.

Go easy.  One only needs to check for failure before anything is done with the parameters.  So a sequence of load calls followed by a single failure check is to be preferred to a sequence of load, failure check pairs.  It's more concise and more efficient.

> 
> 
> Thanks,
> Alistair
> 
> 
>> Levente
>> 
>> 
>>>        fdPointer := interpreterProxy stackValue: 1.
>>>        (interpreterProxy isIntegerObject: fdPointer)
>>>                ifFalse: [^ interpreterProxy primitiveFailFor:
>>> PrimErrBadArgument].
>>>        fd := interpreterProxy integerValueOf: fdPointer.
>>>        filePointer := self connectToFd: fd write: writeFlag.
>>> +       interpreterProxy failed ifFalse:
>>> +               [interpreterProxy pop: 3 "rcvr, name, writeFlag"
>>> +                                                       thenPush:
>>> filePointer]!
>>> -       interpreterProxy failed
>>> -               ifFalse: [^interpreterProxy pop: 3 "rcvr, name, writeFlag"
>>> -                       thenPush: filePointer].
>>> -       ^interpreterProxy primitiveFail.!