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

[Zhengyu Gu]

On Thu, 16 Jan 2025 01:05:51 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Zhengyu Gu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Assertion in wrong place
>
> 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.

This is the scope where `ResourceMark` is needed. no?

> 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() {

I don't mind additional comment, but I prefer to keep assertion as well.

> 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.

I think it is right scope.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1918616476
PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1918619038
PR Review Comment: https://git.openjdk.org/jdk/pull/23121#discussion_r1918619616