<i18n dev> RFR: 8345325: SM cleanup of GetPropertyAction in java.base [v4]

[Roger Riggs]

On Tue, 3 Dec 2024 14:53:41 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> wrote:

>> Roger Riggs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove an obsolete comment related to long ago removed useNewThrowable
>
> src/java.base/share/classes/java/lang/StackStreamFactory.java line 84:
> 
>> 82:     @Native private static final int FILL_LIVE_STACK_FRAMES    = 0x100;
>> 83: 
>> 84:     static final boolean isDebug =
> 
> This property should probably be compared with true in a case insensitive manner. It may be better to use `Boolean::getBoolean` instead.
> 
> Justification:
> 
> JDK-8155775 (commit e8cd76568da1b32b26491e80f498cff1409336b7) removed the `getProperty` method which was previously used to read this property:
> 
> 
> private static boolean getProperty(String key, boolean value) {
>         String s = GetPropertyAction.getProperty(key);
>         if (s != null) {
>             return Boolean.parseBoolean(s);
>         }
>         return value;
>     }
> 
> 
> `isDebug` was initialized with a false default if the system property was null:
> 
> 
> final static boolean isDebug = getProperty("stackwalk.debug", false);
> ``` 
> 
> I doubt the change in case handling was intentional, @cl4es may confirm.
> 
> Based on the above, I think we should consider using Boolean.getBoolean here, as that is case-insensitive and returns false for null:
> 
> Suggestion:
> 
>     static final boolean isDebug = Boolean.getBoolean("stackwalk.debug");

Changing the behavior is out of scope for this pr.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22497#discussion_r1867886415