On Tue, 3 Dec 2024 14:53:41 GMT, Eirik Bjørsnøs <[email protected]> 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
