On Wed, 16 Apr 2025 03:24:26 GMT, Stuart Marks <sma...@openjdk.org> wrote:

>> Remove the #ifdef MACOSX from platform-independent System.c, which (on Mac) 
>> sets native.encoding from sprops.encoding or (on non-Mac Unix and Windows) 
>> sets native.encoding from sprops.sun_jnu_encoding. After this removal, the 
>> native.encoding property will be set from sprops.encoding and the 
>> sun.jnu.encoding property will be set from sprops.sun_jnu_encoding.
>> 
>> Change the windows java_props_md.c so that it initializes 
>> sprops.sun_jnu_encoding and then copies that value to sprops.encoding. 
>> Previously, sprops.encoding was initialized with data from other Windows 
>> APIs, but this value was ignored by platform-independent code, which was 
>> kind of confusing.
>> 
>> There are no changes to the Unix (Mac & non-Mac) platform-specific code. It 
>> already has code to set sprops.encoding and sprops.sun_jnu_encoding 
>> properly, including macOS-specific code to set the latter to UTF-8 
>> unconditionally.
>> 
>> There should be no behavior changes on any platform.
>> 
>> I changed variable names in windows/java_props_md.c to make it clear that 
>> the values stored into them are unused. Of course there are other 
>> possibilities here, such as changing the interface to SetupI18nProps(). I'm 
>> open to suggestions.
>
> Stuart Marks has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains four additional 
> commits since the last revision:
> 
>  - Move asserts into platform-independent code.
>  - Merge branch 'master' into JDK-8354464-additional-cleanup-native-encoding
>  - Rename variables to make clear they're not used.
>  - 8354464: Additional cleanup setting up native.encoding

Changes requested by rriggs (Reviewer).

src/java.base/windows/native/libjava/java_props_md.c line 663:

> 661:                            &sprops.format_country,
> 662:                            &sprops.format_variant,
> 663:                            &format_encoding_unused);

A cleaner change would be to modify SetupI18nProps to remove the encoding 
argument. 
SetupI18nProps is a file local static function, there are no other uses.
And drop the dummy variables.

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

PR Review: https://git.openjdk.org/jdk/pull/24607#pullrequestreview-2772810412
PR Review Comment: https://git.openjdk.org/jdk/pull/24607#discussion_r2047099778

Reply via email to