On Mon, 7 Apr 2025 23:43:29 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> Some of the code that creates various encoding properties at JVM >> initialization time, such as file.encoding and native.encoding, could use >> some cleaning up. Cleanup is fairly minimal and should be mostly >> behavior-preserving. Changes include the following: >> >> * In the java_props.h and java_props_md.c files, add documentation and >> asserts that the sprops.encoding and sprops.sun_jnu_encoding members are >> always set to non-NULL by the platform-specific code. >> >> * In SystemProps.java, remove a null check that is now extraneous based on >> the above assertion. >> >> * In SystemProps.java, rearrange logic (with no behavior change) around >> handling of the "COMPAT" value for the file.encoding property. >> >> * In SystemProps.Raw, rename the platform properties array index from >> _file_encoding_NDX to _native_encoding_NDX. >> >> * In SystemProps.Raw.cmdProperties(), adjust the HashMap size computation. >> This should actually avoid resizing of the HashMap (unlike the earlier code). >> >> * Adjust various comments in several places for clarity and correctness. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Fix comment from file.encoding to native.encoding Renaming the index is fine, the comments/asserts look okay, I assume you've bump the copyright headers before integrating. src/java.base/share/classes/jdk/internal/util/SystemProps.java line 79: > 77: // Platform defined encodings cannot be overridden on the command > line > 78: put(props, "sun.jnu.encoding", > raw.propDefault(Raw._sun_jnu_encoding_NDX)); > 79: var nativeEncoding = raw.propDefault(Raw._native_encoding_NDX); I'd prefer not see "var" here, only because it's not immediately clear that nativeEncoding is a String. ------------- Marked as reviewed by alanb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/24463#pullrequestreview-2755828614 PR Review Comment: https://git.openjdk.org/jdk/pull/24463#discussion_r2036814492
