On Mon, 2 Dec 2024 12:13:57 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

> Can I please get a review of this change which removes usages of 
> SecurityManager related APIs and some leftover related to SecurityManager 
> changes?
> 
> This addresses https://bugs.openjdk.org/browse/JDK-8345286. Most of these 
> changes are trivial. The 
> `src/java.base/linux/classes/jdk/internal/platform/CgroupUtil.java` used to 
> expose utility methods for dealing with SecurityManager permissions and it 
> was called from a few places. That class is no longer needed with the clean 
> up done in this PR.
> 
> No new tests have been introduced and tier testing is currently in progress.

Good cleanup, a few small nits spotted along the way.

src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemController.java
 line 68:

> 66: 
> 67:         try (BufferedReader bufferedReader =
> 68:                          
> Files.newBufferedReader(Paths.get(controller.path(), param))) {

The formatting has got messed up here. If you create `Path path = 
Path.of(controller.path(), param)` then the try line would fit on one line and 
would fix the formatting issue. Maybe some future cleanup will replace this 
with `Files.lines` as this just needs to return the first line.

src/java.base/linux/classes/jdk/internal/platform/CgroupSubsystemController.java
 line 167:

> 165:         if (controller == null) return defaultRetval;
> 166: 
> 167:         try (Stream<String> lines = 
> Files.lines(Paths.get(controller.path(), param))) {

Using Path.of might be clearer here.

src/java.base/share/classes/sun/reflect/misc/MethodUtil.java line 36:

> 34: import java.security.CodeSource;
> 35: import java.security.PermissionCollection;
> 36: import java.security.PrivilegedExceptionAction;

I'm half tempted to suggest leaving MethodUtil out of this change. There is 
further work required her and leaving the SM usage is a reminder of that.

src/java.management/share/classes/sun/management/VMManagementImpl.java line 249:

> 247: 
> 248:         // construct PerfInstrumentation object
> 249:         Perf perf =  Perf.getPerf();

An extra space crept in that at some point

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

Marked as reviewed by alanb (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22478#pullrequestreview-2472587775
PR Review Comment: https://git.openjdk.org/jdk/pull/22478#discussion_r1865783213
PR Review Comment: https://git.openjdk.org/jdk/pull/22478#discussion_r1865783907
PR Review Comment: https://git.openjdk.org/jdk/pull/22478#discussion_r1865778712
PR Review Comment: https://git.openjdk.org/jdk/pull/22478#discussion_r1865785944

Reply via email to