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.

Sign up for GitHub

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

Jump to bottom

Fixes #18141, eliminating null in value when iterating over ENV. #18144

Merged
merged 2 commits into from
Aug 20, 2016
Merged

Fixes #18141, eliminating null in value when iterating over ENV. #18144

merged 2 commits into from
Aug 20, 2016

Conversation

twadleigh
Copy link
Contributor

Fixes #18141.

@tkelman tkelman added system:windows Affects only Windows needs tests Unit tests are required for this change backport pending 0.5 labels Aug 19, 2016
@tkelman
Copy link
Contributor

tkelman commented Aug 19, 2016

Could you add a test that would have failed without this fix?

@twadleigh
Copy link
Contributor Author

@tkelman, will do.

@tkelman tkelman removed the needs tests Unit tests are required for this change label Aug 19, 2016
tkelman
tkelman reviewed Aug 19, 2016
View reviewed changes
test/env.jl

# Test for #18141
for (k, v) in ENV
@test v[end] != '\0'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether it's possible to have this happen from outside of Julia?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could anyone have this be the case in their pre-existing environment, or would our getenv always be reading based on null-termination?

@tkelman tkelman merged commit 03e7c79 into JuliaLang:master Aug 20, 2016
tkelman pushed a commit that referenced this pull request Aug 20, 2016
@twadleigh @tkelman
Fixes #18141, eliminating null in value when iterating over ENV. (#…
293f63a 
…18144)

* Eliminate trailing null from iterator over `ENV`.

Fixes #18141.

* Add failing test for #18141.

(cherry picked from commit 03e7c79)
@wookay wookay mentioned this pull request Aug 25, 2016
Closed
@twadleigh twadleigh deleted the patch-1 branch September 5, 2016 15:27
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
@twadleigh @mfasi
Fixes JuliaLang#18141, eliminating null in value when iterating over …
f238e5b 
…`ENV`. (JuliaLang#18144)

* Eliminate trailing null from iterator over `ENV`.

Fixes JuliaLang#18141.

* Add failing test for JuliaLang#18141.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
system:windows Affects only Windows
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@twadleigh @tkelman