On Mon, 8 Aug 2022 05:22:28 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> JLI_ReportErrorMessageSys has a number of issues, as listed below: >> >> - The windows variant prints message, then extra-info if available, but the >> Unix variant prints first extra-info, then message on a newline. Standardize >> both to print their system errors on newlines below the message. >> >> - The Windows implementation intermingles GetLastError and errno, which is >> incorrect. Both can be set, from independent API calls, and without knowing >> which API was called we don't know which to display. There is no way for >> JLI_ReportErrorMessageSys to know which is the right code. As it is, in its >> current form, we may call JLI_ReportErrorMessageSys for a CRT error and it >> may accidentally display a Win32 API error lingering around from some >> earlier unrelated Win32 API call. Changing JLI_ReportErrorMessageSys to add >> Windows specific components is not desired, so as a compromise we can simply >> print both Win32 and C Runtime errors together, if both exist. >> >> - On macOS, we have two call sites of JLI_ReportErrorMessageSys where the >> errno text is already provided as part of the message by the caller, and >> therefore would be printed twice. >> >> - Visual separation of message and extra-message: the rule is apparently >> that the caller has to provide the separation formatting: "it's up to the >> calling routine to correctly format the separation of messages" (of message >> and extra info). This leads to very awkward call sites (see >> https://github.com/openjdk/jdk/pull/9749). The problem is that the extra >> info may not be available: errno or GetLastError may report 0. That cannot >> be predicted by the caller, therefore the caller should not have to think >> about it. The function should append the extra-info only if available, omit >> it if it is not available, and take care of separation itself. >> >> Side note: The javaw implementation in this patch for Windows is a bit of a >> mess, advice or improvements to it are appreciated > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > I feel stupid src/java.base/unix/native/libjli/java_md_common.c line 209: > 207: /* > 208: * TODO: its safer to use strerror_r but is not available on > 209: * Solaris 8. Until then.... We are well past then :) so this should be changed to use strerror_r - though see ./java.base/unix/native/libjava/jni_util_md.c ------------- PR: https://git.openjdk.org/jdk/pull/9793
