"jerry gay" <[EMAIL PROTECTED]> wrote:
Any input on why we have the tests right shift by 8 bits what spwanw gives them back would be helpful. If its that the exit code on a UNIXy platform is in the upper byte, then surely it's better to right shift it in the platform
specific stuff (in platform.c), so that an exit code of 1 comes back from
spawnw as 1, not 1 << 8?  I'll happily send it a patch to make this so if
there's agreement it's the Right Thing.

what spawned processes return was never fully specified, so this test
has remained broken on windows for a long time. your guess is correct;
the exit code is the upper byte in unix-land (how silly.)

I'm sure there's a reason for it, though I can't think what.

if i recall correctly, the options here are: 1) unix-like return value
from spawnw everywhere, 2) return platform-specific value, or 3)
return some object-like thingy.

i don't like option 1, as i believe spawnw should return something
platform-specific. an object-like thingy (option 3) might be nice, and
is definitely shiny, but it doesn't exist, and requires more
specification.

Sorry, I wasn't clear in what I wrote. I was suggesting a 4th option - always return the exit code so the user can use it right away, as in we >> 8 it on UNIX platforms, leave it as is on Win32, etc. So if the exit code is 1, then Ix contains 1. No matter which platform you're on. User code has to do no shifts.

Part of the point of writing a VM is to hide all that it's appropriate to hide about the underlying OS and platform to stuff at a bytecode level. This is one thing we should be hiding. I don't want my bytecode that uses spawnw to work on UNIX just fine, but break on Win32. Which is the current situation if we leave out the thing we've got now to do option 1, or if we return whatever the OS gives us back without touching it.

in the meantime, you've (happily!) volunteered to implement option 2,
which i believe is the Right Thing. if nobody else disagrees, i'll not
apply this patch, and instead wait for your new patch which gets
spawnw working in a platform-specific manner.

Seems we slight disagree on what is the Right Thing. I'm going to be away for a few days too (visiting family, seeing Rammstein play) so I suggest apply this now and if there's agreement on what to do when I return, I'll implement the other changes then.

Thanks,

Jonathan

Reply via email to