> 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 don't even know anymore ------------- Changes: - all: https://git.openjdk.org/jdk/pull/9793/files - new: https://git.openjdk.org/jdk/pull/9793/files/6175df6e..280942d0 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=9793&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=9793&range=04-05 Stats: 7 lines in 1 file changed: 0 ins; 4 del; 3 mod Patch: https://git.openjdk.org/jdk/pull/9793.diff Fetch: git fetch https://git.openjdk.org/jdk pull/9793/head:pull/9793 PR: https://git.openjdk.org/jdk/pull/9793
