RFR: 8353727: HeapDumpPath doesn't expand %p [v2]

[Thomas Stuefe]

On Tue, 8 Apr 2025 11:43:20 GMT, Kevin Walls <kevinw at 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