On Tue, 6 May 2025 10:56:54 GMT, Thomas Stuefe <stu...@openjdk.org> wrote:
>> Aleksey Shipilev has updated the pull request with a new target base due to >> a merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains six additional >> commits since the last revision: >> >> - Add bug ID >> - Merge branch 'master' into JDK-8352533-jspawnhelper-ioexceptions >> - Pull message into a field >> - Test cases >> - Merge branch 'master' into JDK-8352533-jspawnhelper-ioexceptions >> - Initial fix >> >> Good spawnhelper failure message >> >> Trying tests >> >> Print helper message on all IOExceptions >> >> Final > > src/java.base/unix/native/libjava/ProcessImpl_md.c line 348: > >> 346: } >> 347: /* ASCII Decimal representation uses 2.4 times as many bits as >> binary. */ >> 348: fmtsize = sizeof(IOE_FORMAT) + strlen(internalErrorDetail) + >> strlen(detail) + 3 * sizeof(errnum); > > Suggestion: > > fmtsize = sizeof(IOE_FORMAT) + 3 * sizeof(errnum) + strlen(detail) + > strlen(internalErrorDetail) + 1; > > ... just to appease my OCD. Right. Will be done in new commit. > src/java.base/unix/native/libjava/ProcessImpl_md.c line 351: > >> 349: errmsg = NEW(char, fmtsize); >> 350: if (errmsg == NULL) >> 351: return; > > Preexisting: seems to be weird to just quietly swallow the allocation error. Yes. I am guessing the allocation error here means we are likely unable to allocate the `IOException` later on. History shows this check was added by JDK-8008118 -- and it looks only to please the static analysis tools. So I left this unchanged. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24149#discussion_r2086340557 PR Review Comment: https://git.openjdk.org/jdk/pull/24149#discussion_r2086340234
