Hi Holger,

OK, I made a new VM with your PR after I reviewed it. Good news? It
works!!!  If you want to check OSSubprocess, I am using now the
primCreatePipe from OSProcess because that would answer me directly the
SQFiles of the pipe. Originally (before ending up doing that), I was trying
to make the pipes myself via FFI ( pipe() ) but I came to the problem we
discussed earlier (remember the #name:attachTo:writable:).

So now I can go back to my original solution. The code is in #
*makePipeWithReadBlocking*: and now it looks like this:

| pipePointer returnValue fileDescriptors pipe fileIDsArray fileDescriptor1
fileDescriptor2 |
pipePointer := ExternalAddress allocate: 8.
[
returnValue := self primitivePipe: pipePointer.
(returnValue = -1) ifTrue: [ self perror: 'pipe()' ].
fileIDsArray := Array new: 2.
fileDescriptor1 := pipePointer nbUInt32AtOffset: 0.
fileDescriptor2 := pipePointer nbUInt32AtOffset: 4.
fileIDsArray at: 1 put: *(self primitiveFileOpenUseFileDescriptor:
fileDescriptor1 writeFlag: false)*.
fileIDsArray at: 2 put: *(self primitiveFileOpenUseFileDescriptor:
fileDescriptor2 writeFlag: true).*
pipe := OSSPipe newWith: fileIDsArray readBlocking: aBoolean.
] ensure:[
pipePointer free.
].
^ pipe


I just run all OSSubprocess tests and they all worked! (tested in Pharo
5.0).

I guess I will commit this on the dev branch and hopefully when this is
integrated into the VM I can merge that for my next OSSubprocess release.

Thank you very much Holger




On Thu, Sep 22, 2016 at 12:35 PM, Mariano Martinez Peck <
marianop...@gmail.com> wrote:

>
>
> On Thu, Sep 22, 2016 at 12:12 PM, Holger Freyther <hol...@freyther.de>
> wrote:
>
>>
>> > On 21 Sep 2016, at 15:09, Mariano Martinez Peck <marianop...@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > Exactly. I have been wanting this a couple of times while doing
>> OSSubprocess.
>>
>>
>> https://github.com/pharo-project/pharo-vm/pull/108. Would be nice if you
>> could review it and give it a try. It adds two primitive (one to work on fd
>> one to work on FILE).
>>
>> I probably also want to do:
>>
>>         sqFile->isStdioStream = isatty(fileno(file))
>>
>>
>> > Yes, exactly. I remember now. And as I said, I also wanted to be able
>> to work at fd or FILE* level and I failed.
>>
>> Could you give the above a try and then I try to get it into the
>> Opensmalltalk VM.
>>
>
> Hi Holger,
>
> I just reviewed the PR and it looks really good. Please, allow me some
> time to get updated to the VM compiling instructions and I will give it a
> try. Probably, I will be testing it by doing my original (unused now)
> #name:attachToCFile:writable:  use the new primitive and running the
> OSSubprocess tests. Does this make sense to you?
>
> Thanks a LOT for going deep and fix it!
>
>
>
>
> --
> Mariano
> http://marianopeck.wordpress.com
>



-- 
Mariano
http://marianopeck.wordpress.com

On Thu, Sep 22, 2016 at 12:35 PM, Mariano Martinez Peck <
marianop...@gmail.com> wrote:

>
>
> On Thu, Sep 22, 2016 at 12:12 PM, Holger Freyther <hol...@freyther.de>
> wrote:
>
>>
>> > On 21 Sep 2016, at 15:09, Mariano Martinez Peck <marianop...@gmail.com>
>> wrote:
>> >
>> >
>> >
>> > Exactly. I have been wanting this a couple of times while doing
>> OSSubprocess.
>>
>>
>> https://github.com/pharo-project/pharo-vm/pull/108. Would be nice if you
>> could review it and give it a try. It adds two primitive (one to work on fd
>> one to work on FILE).
>>
>> I probably also want to do:
>>
>>         sqFile->isStdioStream = isatty(fileno(file))
>>
>>
>> > Yes, exactly. I remember now. And as I said, I also wanted to be able
>> to work at fd or FILE* level and I failed.
>>
>> Could you give the above a try and then I try to get it into the
>> Opensmalltalk VM.
>>
>
> Hi Holger,
>
> I just reviewed the PR and it looks really good. Please, allow me some
> time to get updated to the VM compiling instructions and I will give it a
> try. Probably, I will be testing it by doing my original (unused now)
> #name:attachToCFile:writable:  use the new primitive and running the
> OSSubprocess tests. Does this make sense to you?
>
> Thanks a LOT for going deep and fix it!
>
>
>
>
> --
> Mariano
> http://marianopeck.wordpress.com
>



-- 
Mariano
http://marianopeck.wordpress.com

Reply via email to