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

[David Holmes]

On Wed, 15 Jan 2025 19:22:50 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:
> 
>   Assertion in wrong place

Thanks for updating to remove the copying. A few minor nits to address.

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

> 642: 
> 643: void Modules::serialize_native_access_flags(SerializeClosure* soc) {
> 644:   ResourceMark rm;

I would suggest moving this to the scope where it is needed.

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

> 660: }
> 661: 
> 662: const char* Modules::get_addmods_names_as_sorted_string() {

Suggestion:

// Caller needs ResourceMark
const char* Modules::get_addmods_names_as_sorted_string() {

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

> 666: 
> 667: void Modules::serialize_addmods_names(SerializeClosure* soc) {
> 668:   ResourceMark rm;

Again I would suggest moving this to the scope where it is needed.

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

> 676: }
> 677: 
> 678: const char* Modules::get_numbered_property_as_sorted_string(const char* property) {

Suggestion:

// Caller needs ResourceMark
const char* Modules::get_numbered_property_as_sorted_string(const char* property) {

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

Changes requested by dholmes (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23121#pullrequestreview-2554461753
PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1917528401
PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1917534470
PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1917528897
PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1917531358