On Tue, 8 Apr 2025 11:43:20 GMT, Kevin Walls <kev...@openjdk.org> wrote:
>> This is a long-standing oversight: HeapDumpPath does not recognise %p for
>> pid expansion.
>> The default filename uses a pid (e.g. java_pid1676937.hprof) but
>> HeapDumpPath does not.
>> It has always done a manual "root plus pid plus extension" on the default
>> filename only, and
>> should move to using Argument::copy_expand_pid() like we do with other such
>> filenames.
>>
>>
>> We also assumed the default filename is not a directory (which is very very
>> likely, but doesn't have to be true).
>
> Kevin Walls has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - length checking update
> - Chris feedback
src/hotspot/share/services/heapDumper.cpp line 2760:
> 2758: if (dump_file_seq == 0) { // first time in, we initialize base_path
> 2759: // Set base path (name or directory, default or custom, without seq
> no), doing %p substitution.
> 2760: const char *path_src = (HeapDumpPath != nullptr && HeapDumpPath[0]
> != '\0') ? HeapDumpPath : dump_file_name;
Why do you expand the dump file name here?
If you want to minimize the expand calls, you could:
- append the unexpanded dump_file_name to the unexpanded HeapDumpPath
- expand
- create dir (extract the directory name by temporarily setting the the last
'/' to '\0; create dir; restore '/')
now you are done.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24482#discussion_r2034576102
