RFR: 8349254: Disable "best-fit" mapping on Windows environment variables [v2]

[Justin Lu]

On Fri, 7 Feb 2025 20:11:06 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> src/java.base/share/native/libjli/args.c line 611:
>> 
>>> 609:         LPWSTR wcVarName = JLI_MemAlloc(wcCount * sizeof(wchar_t));
>>> 610:         if (MultiByteToWideChar(CP_ACP, 0, var_name, -1, wcVarName, wcCount) != 0) {
>>> 611:             LPWSTR wcEnvVar = _wgetenv(wcVarName);
>> 
>> Since we are in Windows specific code, would it make sense to call `GetEnvironmentVariableW` over `_wgetenv`? It won't be as concise since we will have to follow the 2-call style of determining the size, then filling the buffer; but I presume it avoids some overhead, since it's directly apart of the Win32 API?
>
> I think the overhead is negligible. If we use the Get... function, we will need to allocate/deallocate the intermediate buffer, which will make the code complex.

OK, seems good to use `_wgetenv` then.

>> test/jdk/tools/launcher/DisableBestFitMappingTest.java line 32:
>> 
>>> 30:  * @requires (os.family == "windows")
>>> 31:  * @library /test/lib
>>> 32:  * @run junit DisableBestFitMappingTest
>> 
>> I think it might be best to re-write this test as a non Junit test. Through IntelliJ it's hard to run this test, I presume because of the combination of it being a Junit test and having a `main` method? If I run the 'launcher' suite of tests, this test does not seem to be included.
>
> I am not sure of this. Isn't it the issue in jtreg plugin? At least `make test TEST=...` will succeed.

Oops, I thought the test would mark as "skipped" or something like that, but yes its a Windows only test, hence why it does not show up, duh.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23498#discussion_r1947288085
PR Review Comment: https://git.openjdk.org/jdk/pull/23498#discussion_r1947288095