RFR: 8354464: Additional cleanup setting up native.encoding [v2]

Naoto Sato naoto at openjdk.org
Wed Apr 16 20:48:46 UTC 2025


On Wed, 16 Apr 2025 17:44:36 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> 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.
>
> The main focus of this commit is to straighten out the logic between the platform-specific and platform-independent layers.
> 
> In the previous state of this file, tracing the origin of the properties on Windows, I ended up following the origin of sprops.encoding all the way through SetupI18nProps, only to find that was ignored entirely at the platform-independent layer. So this change is an improvement in that it clarifies where sprops.encoding comes from, and that SetupI18nProps does some computations that end up being thrown away instead of being stored into a shared data structure that's eventually ignored.
> 
> Of course that path is dead code and could be removed without changing the behavior. But this code has a tortured history and it's not entirely clear that it's correct in its current state. In fact @naotoj considered making some changes in this logic [JDK-8352917](https://bugs.openjdk.org/browse/JDK-8352917) but decided against it. However it seems like a possibility that we might revisit this?
> 
> Also note that we're now talking about how Windows gets its encoding information from the OS, and not how encoding information is transmitted up through the layers into Java properties, which is the focus of this cleanup.
> 
> Anyway I'd like to hear from @naotoj on this. If we're certain the extra stuff in SetupI18nProps is useless, we should clean it up now. But if there might be a use for this information -- maybe stdin.encoding? -- then perhaps it's better to leave it as is.

I belive they can be removed as they are no where used (and I don't think it will be used for `stdin.encoding` either) So I prefer removing those unused code now.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24607#discussion_r2047728357


More information about the core-libs-dev mailing list