RFR: 8285517: System.getenv() returns unexpected value if environment variable has non ASCII character [v2]

[Ichiroh Takiguchi]

On Tue, 26 Apr 2022 21:17:34 GMT, Naoto Sato <naoto at openjdk.org> wrote:

>> Ichiroh Takiguchi has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8285517: System.getenv() returns unexpected value if environment variable has non ASCII character
>
> Thanks for fixing the issue.
> As to the test, it has lots of redundancy, and too complicated to me. Can you simplify the test?

Hello @naotoj and @RogerRiggs .
I appreciate your comments.
I applied the changes.
Additionally, I put bugid into test/jdk/java/lang/ProcessBuilder/Basic.java

> src/java.base/unix/classes/java/lang/ProcessImpl.java line 146:
> 
>> 144:     private static final LaunchMechanism launchMechanism = platform.launchMechanism();
>> 145:     private static final byte[] helperpath = toCString(StaticProperty.javaHome() + "/lib/jspawnhelper");
>> 146:     private static Charset jnuCharset = null;
> 
> Can we simply call `jnuCharset()` here?

Change jnuCharset to "private static".

> test/jdk/java/lang/ProcessBuilder/Basic.java line 605:
> 
>> 603:             String fileEncoding = System.getProperty("file.encoding");
>> 604:             Charset cs;
>> 605:             if (nativeEncoding != null && !nativeEncoding.equals(fileEncoding)) {
> 
> What's the reason for comparing `file.encoding`? Does `Charset.forName(nativeEncoding, Charset.defaultCharset())` suffice?

Remove file.encoding related code.

> test/jdk/java/lang/System/i18nEnvArg.java line 26:
> 
>> 24: /*
>> 25:  * @test
>> 26:  * @bug 8285517
> 
> If the test should work only on a particular env, should describe here and add `@requires`  tag.

Add "@requires (os.family == "linux")"

> test/jdk/java/lang/System/i18nEnvArg.java line 50:
> 
>> 48: 
>> 49:     public static void main(String[] args) throws Exception {
>> 50:         ProcessBuilder pb = new ProcessBuilder(javeExe,
> 
> Can utilize `jdk.test.lib.process.ProcessTools`.

Use ProcessTools class.

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

PR: https://git.openjdk.java.net/jdk/pull/8378