On Tue, 10 Nov 2026 02:58:49 GMT, Chris Plummer <[email protected]> wrote:
>> Yasumasa Suenaga has updated the pull request incrementally with four
>> additional commits since the last revision:
>>
>> - Remove blank line
>> - improvement
>> - Fix
>> - Exclude libDebuginfoTest.c from non-Linux platforms
>
> src/jdk.hotspot.agent/linux/native/libsaproc/salibelf.c line 253:
>
>> 251: if (read(fd, buf, php->p_filesz) != php->p_filesz) {
>> 252: print_error("failed to read PT_NOTE data in %s\n", filename);
>> 253: break;
>
> You should free buf first.
Thanks, fixed.
> src/jdk.hotspot.agent/linux/native/libsaproc/salibelf.c line 275:
>
>> 273: print_error("failed to allocate memory for Build ID of
>> %s\n", filename);
>> 274: }
>> 275: loop_exit = true;
>
> Setting loop_exit here does not break out of the inner loop. If another match
> is found on line 267, you will leak the previously malloc'd *buildid.
I expect to exit loop when Build ID was found, so I added `break` here.
> test/hotspot/jtreg/serviceability/sa/TestELFDebuginfo.java line 44:
>
>> 42: * @run driver TestELFDebuginfo
>> 43: */
>> 44: public class TestELFDebuginfo {
>
> This test needs comments describing its purpose, and describing how it
> relies on the build changes you made to create a .debuginfo that is stripped
> of the .note.gnu.build-id section.
I added `@summary` and left comments what we test, and expect.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/33764#discussion_r4521244280
PR Review Comment: https://git.openjdk.org/jdk/pull/33764#discussion_r4521254433
PR Review Comment: https://git.openjdk.org/jdk/pull/33764#discussion_r4521265811
