OSProcess chdir

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

OSProcess chdir

Sean P. DeNigris
Administrator
UnixOSProcessPlugin>>primitiveChdir
...
        (self chdir: path)
                ifTrue: [interpreterProxy pop: 2; push: interpreterProxy nilObject]
                ifFalse: [interpreterProxy pop: 2; pushInteger: errno].

Shouldn't the conditional branches be reversed (or the test be "(self chdir: path) == 0" - however you would write that in Slang)? chdir returns 0 on success and -1 on error. In c, the above becomes:
        if (chdir(path)) {...interpreterProxy->push(interpreterProxy->nilObject());}
        else {...interpreterProxy->pushInteger(errno);}
So success takes the else branch and returns the irrelevant errno, which I guess is why I'm getting confusing error codes in Pharo 2.0 even though the chdir has taken effect.

b.t.w. if the above is correct, how would I submit a patch (i.e. changes to the UnixOSProcessPlugin class)?

Thanks,
Sean
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: OSProcess chdir

David T. Lewis
 
On Fri, May 25, 2012 at 11:33:17AM -0700, Sean P. DeNigris wrote:

>  
> UnixOSProcessPlugin>>primitiveChdir
> ...
> (self chdir: path)
> ifTrue: [interpreterProxy pop: 2; push: interpreterProxy nilObject]
> ifFalse: [interpreterProxy pop: 2; pushInteger: errno].
>
> Shouldn't the conditional branches be reversed (or the test be "(self chdir:
> path) == 0" - however you would write that in Slang)? chdir returns 0 on
> success and -1 on error. In c, the above becomes:
> if (chdir(path))
> {...interpreterProxy->push(interpreterProxy->nilObject());}
> else {...interpreterProxy->pushInteger(errno);}
> So success takes the else branch and returns the irrelevant errno, which I
> guess is why I'm getting confusing error codes in Pharo 2.0 even though the
> chdir has taken effect.

Yes this is a bug.

>
> b.t.w. if the above is correct, how would I submit a patch (i.e. changes to
> the UnixOSProcessPlugin class)?

Just a change set is usually the easiest thing, although in this case
the bug is already fixed in the sources (Damien Cassou spotted the same
thing recently).

The fixed version is OSProcessPlugin versionString 4.4.11 in
VMConstruction-Plugins-OSProcessPlugin-dtl.35.mcz on SqueakSource/OSProcessPlugin.
And Eliot picked up the fix for Cog in VMConstruction-Plugins-OSProcessPlugin.oscog-eem.35.mcz.

Thanks!
Dave

Reply | Threaded
Open this post in threaded view
|

Re: OSProcess chdir

Sean P. DeNigris
Administrator
David T. Lewis wrote
Just a change set is usually the easiest thing
Okay, great... where should the cs be sent (for next time)?
Cheers,
Sean
Reply | Threaded
Open this post in threaded view
|

Re: OSProcess chdir

David T. Lewis
 
On Fri, May 25, 2012 at 04:08:49PM -0700, Sean P. DeNigris wrote:
>  
>
> David T. Lewis wrote
> >
> > Just a change set is usually the easiest thing
> >
>
> Okay, great... where should the cs be sent (for next time)?
>

You can send it to me and/or squeak-dev and/or vm-dev. I try to keep an
eye on pharo lists also. Eliot has developer access to the repository in
case I get run over by a truck, and I'll be happy to add you also if you
have updates to post (but realistically OSPP is very stable, the main
need for updates would be for the Windows OSPP plugin, which I have not
gotten around to writing in lo these many years).

Dave