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