[crac] RFR: Print better diagnostics for spawning CRIU [v2]
Anton Kozlov
akozlov at openjdk.org
Thu Aug 10 12:00:58 UTC 2023
On Wed, 9 Aug 2023 22:45:26 GMT, Jan Kratochvil <jkratochvil at openjdk.org> wrote:
>> src/java.base/unix/native/criuengine/criuengine.cpp line 91:
>>
>>> 89: }
>>> 90: }
>>> 91: retval += '\'';
>>
>> It will be surprised to meet these symbols in the args, and IMHO this is overly complicated way to report them
>
> Should it therefore abort when it sees such symbol? Otherwise the problem will not be reproducible from the error message.
You mean the line is not copy-pastable. But it is not such anyway, as it makes sense only for specific pid, and in the specific point in time (in VM_Crac safepoint).
I'm fine with the current code.
>> src/java.base/unix/native/criuengine/criuengine.cpp line 109:
>>
>>> 107: free(cwd_s);
>>> 108: return path_from(cwd, rel);
>>> 109: }
>>
>> The problem with this function as abstraction is that it first calculates CWD, then checks was the argument absolute or relative.
>
> The code was more simple that way and it was straightforward to replace it with C++17 `std::filesystem::path` after [OpenJDK makes the switch to C++17](https://bugs.openjdk.org/browse/JDK-8310260).
The interface is fine, the implementation just could be more straightforward and efficient.
The link you're refering has Resolution: Rejected.
-------------
PR Review Comment: https://git.openjdk.org/crac/pull/97#discussion_r1290007985
PR Review Comment: https://git.openjdk.org/crac/pull/97#discussion_r1290006210
More information about the crac-dev
mailing list