RFR: 8347758: modules.cpp leaks string returned from get_numbered_property_as_sorted_string() [v2]

[Calvin Cheung]

On Wed, 15 Jan 2025 14:47:53 GMT, Zhengyu Gu <zgu at openjdk.org> wrote:

>> Modules::get_numbered_property_as_sorted_string() returns strings created from os::strdup(), callers should free them after uses.
>
> Zhengyu Gu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Avoid copying strings

Hi Zhengyu,
Thanks for noticing this and fixing it. I've couple of comments.

src/hotspot/share/classfile/modules.cpp line 575:

> 573: 
> 574: void Modules::check_archived_flag_consistency(char* archived_flag, const char* runtime_flag, const char* property) {
> 575:   assert(Thread::current()->current_resource_mark() != nullptr, "Setup by caller");

Is this necessary? Among the three callers: `serialize`, `serialize_native_access_flags`, and `serialize_addmods_names`, only the latter two need a `ResourceMark`.

src/hotspot/share/classfile/modules.cpp line 618:

> 616: 
> 617: void Modules::serialize(SerializeClosure* soc) {
> 618:   ResourceMark rm;

This is only needed for the `assert` in `check_archived_flag_consistency`.

src/hotspot/share/classfile/modules.cpp line 673:

> 671:   if (soc->reading()) {
> 672:     const char* addmods_names = get_addmods_names_as_sorted_string();
> 673:     check_archived_flag_consistency(_archived_addmods_names, addmods_names, "jdk.module.addmods");

Is this change necessary?

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

PR Review: https://git.openjdk.org/jdk/pull/23121#pullrequestreview-2553570372
PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1917143707
PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1917143877
PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1917144046