Windows: Simplify ENV code by removing compatibility and duplicated code #7034
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
larskanis
force-pushed
the
remove-wputenv
branch
2 times, most recently
from
6d8c733 to
653444f
Compare
larskanis
force-pushed
the
remove-wputenv
branch
3 times, most recently
from
7925ef3 to
c04c4cb
Compare
larskanis
force-pushed
the
remove-wputenv
branch
from
c04c4cb to
adaf8cf
Compare
Using _wputenv_s simplifies the code and we can avoid code duplication by using rb_w32_home_dir() to initialize ENV['HOME'].
Windows Vista removed the limit to 32768 bytes environment block. Also MINGW supports _wputenv_s() for years.
This only makes a difference when setting an empty value to a Unicode key.
hsbt
force-pushed
the
remove-wputenv
branch
from
adaf8cf to
a2e0e23
Compare
|
Thank you @hsbt for rebasing! I still believe that this PR is valuable, especially since it fixes a corner case bug on Windows. |
|
The changes of @nobu Can you review them? |
nobu
approved these changes
Sep 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
_wputenv_sis available in all supported MSVC versions and in MINGW since years. Dropping support for_wputenvwithout_sallows to remove some complexity and duplication, sincerb_w32_home_dir()is usable in the win32 initialization then.Also fix a single ANSI call to
SetEnvironmentVariable()by using the Unicode equivalent. I added a test case for it.