RFR: 8292016: Rework JLI_ReportErrorMessageSys [v10]

[Julian Waters]

On Thu, 18 Aug 2022 13:22:28 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> The (rather incomplete) fix for this was indeed initially part of the original RFE, and only split into another bug afterwards when it was realized there was too much to fit into one PR for an easy review, but I thought it'd be less of a clutter to focus on this one first since the issue already exists in the tracker
>> 
>>> make an RFE that actually enriches failing Win32 API calls with GetLastError, using the non-sys variant (JLI_ReportErrorMessage) and just manually adding the error code at the call sites.
>> 
>> I don't quite get what you mean?
>
>> I don't quite get what you mean?
> 
> The only thing JLI_ReportErrorMessageSys has above JLI_ReportErrorMessage is that it prints the error code for you, and that is a very slim win. It turns into a loss quickly if it does that one thing wrong, e.g. by printing unrelated error codes that confuse the analyst. And if that causes us and you to burn cycles with this discussion that could be spent much better elsewhere.
> 
> Therefore, what I meant to suggest is to fix whatever you originally wanted to fix with your original JBS issue https://bugs.openjdk.org/browse/JDK-8291917 by calling JLI_ReportErrorMessage directly.
> 
> e.g. 
> 
> p = VirtualAlloc(...) or whatever win32 API you want to cover
> if (p == NULL) {
>   JLI_ReportErrorMessage("Failed to allocate xxx (%u)", GetLastError());
> }

Initially I did intend to do that, but a raw GetLastError wouldn't exactly be very helpful when a failure does occur, and unfortunately formatting the proper message from it takes quite a bit of ceremony (Thanks for nothing Win32 API!) that would hopefully be confined to only one function - And it seems so far that this one is quite a suitable candidate. To be sure, that RFE doesn't strictly require this one, and can be changed to use other utilities instead, but as mentioned above the same fault is in Windows error reporting routines all over the JDK, and unlike ReportErrorMessageSys which isn't used that often, those are very commonly used across the codebase, by doing that all we'd be doing is sidestepping the problem and leaving it to be fixed later. I did hope that a warning or outright refusal to concat the errors when either GetLastError and errno != 0 was a good solution, though

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

PR: https://git.openjdk.org/jdk/pull/9870