On Fri, 22 Dec 2023 08:40:08 GMT, Alan Bateman <al...@openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   comments from Alan and Ioi
>
> src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java line 71:
> 
>> 69:         URLClassPath bootAppendUcp = (append != null && 
>> !append.isEmpty())
>> 70:                 ? new URLClassPath(append, true)
>> 71:                 : null;
> 
> Would you mind renaming this to bootUcp or bcp? The reason is that this is 
> the boot loader's class path, not the append path. The system property has 
> "append" in the name as it's communicating the value of -Xbootclasspath/a to 
> the library code, this came about when -Xbootclasspath and -Xbootclasspath/p 
> were removed.

A full bcp would contain the path to the module image as the first path. This 
"append" property only contains the path specified in -Xbootclasspath/a. That's 
why I had bootAppendUcp as the variable name.
I've changed it to `bootUcp` per your suggestion.

> src/java.base/share/classes/jdk/internal/loader/ClassLoaders.java line 76:
> 
>> 74:             BOOT_LOADER = (BootClassLoader) 
>> archivedClassLoaders.bootLoader();
>> 75:             setArchivedServicesCatalog(BOOT_LOADER);
>> 76:             BOOT_LOADER.setClassPath(bootAppendUcp);
> 
> It might be clearer to set this before calling setArchivedServicesCatalog.

Fixed. Also removed the commented assert statement.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17178#discussion_r1435274444
PR Review Comment: https://git.openjdk.org/jdk/pull/17178#discussion_r1435274493

Reply via email to