On Wed, 18 Sep 2024 01:15:40 GMT, David Holmes <[email protected]> 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
