Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed bug on Windows when creating process with empty environment. #2031

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
2 participants
@BenoitJGirard
Copy link
Contributor

BenoitJGirard commented Mar 5, 2019

Apparently CreateProcessW() expects the input environment block to have two null 2-byte characters, even if there are no strings in the block. Not obvious from documentation.

Let me know if more is needed in terms of submitted tests, etc.

BenoitJGirard and others added some commits Mar 5, 2019

Benoit Jauvin-Girard
Make createWindowsEnvBlock() work for empty environment
On Windows, it turns out that CreateProcessW expects an empty
environment block to be composed of 4 null bytes. This was found out
experimentally.

Added the empty environment case to code of
std.os.windows.util.createWindowsEnvBlock().
@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 8, 2019

Thanks for the contribution! I'll look at this today and add a test for it. How did you find this out?

@BenoitJGirard

This comment has been minimized.

Copy link
Contributor Author

BenoitJGirard commented Mar 8, 2019

Found out by adding a CommandStep to a Builder with an empty environment, in an attempt to start a "sanitized" Zig process with all inputs explicit.

Where should I have added a test?

@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 8, 2019

Ahh, that makes sense. If you want to try your hand at making a test, here's my recommendation:

  • create a standalone test in test/standalone/your_new_directory
  • add your new standalone test to test/build_examples.zig (sorry for the misleading name)
  • look at the other standalone tests for an example
  • to run it, do this: bin/zig build --build-file ../build.zig test-build-examples, or you can cd to your new directory and run ../../../build/bin/zig build test

andrewrk added a commit that referenced this pull request Mar 11, 2019

add test for spawning child process with empty environment
thanks to BenoitJGirard for pointing out the child process
implementation needs 3 extra null bytes in #2031
@andrewrk

This comment has been minimized.

Copy link
Member

andrewrk commented Mar 11, 2019

I ended up coding it a different way, and added a test in e1fbb24. So this is now fixed. Thanks again!

@andrewrk andrewrk closed this Mar 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.