[crac] RFR: 8367975: [CRaC] Pattern in CRaCCheckpointTo [v5]

[Timofei Pushkin]

On Tue, 30 Sep 2025 13:39:59 GMT, Radim Vansa <rvansa at openjdk.org> wrote:

>> Add support for pattern in `CRaCCheckpointTo` VM option.
>
> Radim Vansa has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix build

src/hotspot/share/runtime/crac.cpp line 221:

> 219:           int ret = data.print_numbers(buf, buflen, true);
> 220:           buf += ret;
> 221:           buflen -= ret;

We only check that there is enough space for a single character in the buffer while `print_numbers` asserts that there is enough space for all the features to fit. I.e. I believe it is possible to go past the end of the buffer here.

Although not a part of this change, but the assert in `print_numbers` should be before subtracting from `buflen`, not after.

src/hotspot/share/runtime/crac.cpp line 301:

> 299:   // Note that CRaCEngine and CRaCEngineOptions are not updated (as documented)
> 300:   // so we don't need to re-init the whole engine handle.
> 301:     char image_location[PATH_MAX];

Suggestion:

  char image_location[PATH_MAX];

src/hotspot/share/runtime/globals.hpp line 1965:

> 1963:           "image is a directory, the directory will be created if it does " \
> 1964:           "not exist (parent directories are not created) or overwritten "  \
> 1965:           "otherwise. The path can contain placeholders (e.g. %p for PID);" \

Whitespace is missing after ";"

src/java.base/share/man/java.md line 1088:

> 1086:     these placeholders:
> 1087:     -   `%%`: single % character
> 1088:     -   `%a`: architecture (x86_64 or aarch64)

Are you sure it is always one of these? [This code](https://github.com/openjdk/crac/blob/5abb5dac8e9785ad0c38e238e526775f98617aec/make/autoconf/platform.m4#L475) suggests that it can be i386 or amd64 for example. Also the shared part of CRaC is probably compilable for other arches too. Maybe better to say it is the same as what `os.arch` returns?

src/java.base/share/man/java.md line 1095:

> 1093:               separators) with second precision, e.g. `20250909T141711Z`
> 1094:     -   `%T`: checkpoint epoch time (second precision)
> 1095:     -   `%b` and `%B`: process boot time (generation 1), same format as `%t` or `%T`

I suggest putting the description of `%g` before this (between `%u` and `%t`, for example) so that the definition of a "generation" is more clear when the reader reaches this point

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

PR Review Comment: https://git.openjdk.org/crac/pull/264#discussion_r2398115959
PR Review Comment: https://git.openjdk.org/crac/pull/264#discussion_r2398519612
PR Review Comment: https://git.openjdk.org/crac/pull/264#discussion_r2398533814
PR Review Comment: https://git.openjdk.org/crac/pull/264#discussion_r2398429069
PR Review Comment: https://git.openjdk.org/crac/pull/264#discussion_r2398460145