RFR: 8345286: Remove use of SecurityManager API from misc areas [v2]

Jaikiran Pai jpai at openjdk.org
Mon Dec 2 13:39:59 UTC 2024


On Mon, 2 Dec 2024 12:42:47 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> Jaikiran Pai has updated the pull request incrementally with four additional commits since the last revision:
>> 
>>  - remove unnecessary space
>>  - Path.of() instead of Paths.get()
>>  - fix formatting of try-with-resources in CgroupSubsystemController.java
>>  - leave out MethodUtil from the clean up
>
> 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.

Fixed. I moved the `Path.of()` outside of the try-with-resources and the line length is now more reasonable.

> 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.

Done, here and one other place in this file.

> 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.SecureClassLoader;
> 
> 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.

Understood. I've now updated this PR to revert back the changes in this file.

> 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

Thanks for spotting that, I hadn't noticed it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22478#discussion_r1865864725
PR Review Comment: https://git.openjdk.org/jdk/pull/22478#discussion_r1865865592
PR Review Comment: https://git.openjdk.org/jdk/pull/22478#discussion_r1865863888
PR Review Comment: https://git.openjdk.org/jdk/pull/22478#discussion_r1865865984


More information about the core-libs-dev mailing list