RFR: 8328313: Archived module graph should allow identical --module-path to be specified during dump time and run time [v3]

[Calvin Cheung]

On Wed, 18 Sep 2024 01:15:40 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   trailing whitespace
>
> src/hotspot/share/cds/filemap.cpp line 931:
> 
>> 929: bool FileMapInfo::is_jar_suffix(const char* filename) {
>> 930:   const char* dot = strrchr(filename, '.');
>> 931:   if (strcmp(dot + 1, "jar") == 0) {
> 
> What if there is no dot? We need a null check.

Added a null check.

> src/hotspot/share/cds/filemap.hpp line 558:
> 
>> 556:                     unsigned int dumptime_prefix_len,
>> 557:                     unsigned int runtime_prefix_len) NOT_CDS_RETURN_(false);
>> 558:   bool  is_jar_suffix(const char* filename);
> 
> Suggestion: has_jar_suffix

Fixed. Also moved the function to ClassLoaderExt.cpp.

> src/hotspot/share/cds/heapShared.cpp line 884:
> 
>> 882:       ClassLoaderExt::num_module_paths() > 0) {
>> 883:     log_info(cds, heap)("    is_using_optimized_module_handling %d num_module_paths %d jdk.module.main %s",
>> 884:         CDSConfig::is_using_optimized_module_handling(), ClassLoaderExt::num_module_paths(), Arguments::get_property("jdk.module.main"));
> 
> Why are you printing a bool value as an int? I'm surprised one of the format checkers doesn't complain about it.

I changed it to BOOL_TO_STR and updated the log statement.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21048#discussion_r1769018271
PR Review Comment: https://git.openjdk.org/jdk/pull/21048#discussion_r1769018370
PR Review Comment: https://git.openjdk.org/jdk/pull/21048#discussion_r1769018601