On Wed, 16 Apr 2025 17:44:36 GMT, Stuart Marks <sma...@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
