On Fri, 15 Nov 2024 09:35:17 GMT, Eirik Bjørsnøs <eir...@openjdk.org> wrote:

> Please review this PR to clean up `RandomSupport`, `ClassFileDumper` and 
> `StaticProperty` in the `jdk.internal.util` namespace:
> 
> * `RandomSupport` is updated to replace an `AccessController::doPrivileged` 
> call with `Boolean::getBoolean`. (Existing code uses 
> `String::equalsIgnoreCase`, equivalent to `Boolean::getBoolean`)
> * `ClassFileDumper` constructor is updated to remove a comment referencing 
> `GetPropertyAction`. (I left the `VM::getSavedProperty` call as-is, please 
> advise if this should be replaced with `System::getProperty)
> * `ClassFileDumper::write` is updated to unwrap a 
> `AccessController::doPrivileged` call
> * `StaticProperty` is updated to remove 
> `SecurityManager::checkPropertyAccess` references in the documentation
> 
> Verification: GHA results pending.

A few remarks; look fine otherwise.  Pinging @rgiulietti to review the random 
generator changes.

src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java line 129:

> 127:     }
> 128: 
> 129:     @SuppressWarnings("removal")

Please remove this redundant suppression.

src/java.base/share/classes/jdk/internal/util/ClassFileDumper.java line 148:

> 146:      * Validate if the given dir is a writeable directory if exists.
> 147:      */
> 148:     @SuppressWarnings("removal")

Same redundant suppression.

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

Changes requested by liach (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22141#pullrequestreview-2438841002
PR Review Comment: https://git.openjdk.org/jdk/pull/22141#discussion_r1843959825
PR Review Comment: https://git.openjdk.org/jdk/pull/22141#discussion_r1843960575

Reply via email to