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

[Naoto Sato]

On Fri, 7 Feb 2025 18:53:40 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Naoto Sato has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Reflects review
>
> src/java.base/share/native/libjli/args.c line 603:
> 
>> 601: /*
>> 602:  * getenv() without best-fit mapping
>> 603:  */
> 
> I think a quick comment that says something along the lines of 
> 
> 
> env variable name: code page -> UTF-16 
> variable value : UTF-16 -> code page
> 
> 
> would be helpful to overview what is happening here.

Thanks. I modified the comment.

> 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.

> 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.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23498#discussion_r1947128400
PR Review Comment: https://git.openjdk.org/jdk/pull/23498#discussion_r1947128384
PR Review Comment: https://git.openjdk.org/jdk/pull/23498#discussion_r1947128439