On Sun, 17 Nov 2024 14:52:06 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:
> Please review this PR which removes exceptional control flow in > `Boolean::getBoolean`, `Integer::getInteger` and `Long::getLong`. > > These methods are catching `IllegalArgumentException` and > `NullPointerException`, thrown by `System::getProperty` via > `System::checkKey`. This PR replaces the exceptional control flow with > explicit checks that the system property name is non-null and non-empty > before calling into `System::getProperty`. > > As JDK-8178966 points out, there is a possibility that System.getProperty > could throw one of NPE and IAE for other reasons than the name being null or > empty. This risk is reduced now that custom security managers cannot > interfere. Adding to that, if such exceptions are thrown today, they are > masked by these methods catching and swallowing them by returning false or > default values. It's better to expose such bugs if they exist. > > GHA results pending. Local tier2 ran successfully. Circumventing and duplicating the checks in System.getProperty isn't good coding technique and is not necessary for performance improvement or code size. ------------- Changes requested by rriggs (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/22183#pullrequestreview-2442959716