On Thu, 4 Aug 2022 17:42:45 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> Please review a small patch for dumping the failure reason when the MSVCRT >> libraries or the Java Virtual Machine fails to load on Windows, which can >> provide invaluable insight when debugging related launcher issues. This also >> fixes a small bug in the Windows implementation of JLI_ErrorMessageSys where >> C runtime errors are never written to stderr. > > Julian Waters has updated the pull request incrementally with one additional > commit since the last revision: > > Missing spacing between errors Hi Julian, I don't get it. AFAICS the existing version displays the failure reason for win32 API just fine, appending it to the message. Where is the bug? I see that the message was omitted for CRT errors, so that is okay to fix, but that could have been fixed with a simple freeit=1 in the CRT error path (although my taste would be to just append errtxt if errtxt!=NULL, which is less circumvent). I also do not understand the changes to the call sites, where you add "%s". What is the point? If you want to separate error message and detail error message with " - ", you can just do an additional conditional strcat in the error message function. Cheers, Thomas src/java.base/share/native/libjli/emessages.h line 111: > 109: #define DLL_ERROR2 "Error: failed %s, because %s" > 110: #define DLL_ERROR3 "Error: could not find executable %s" > 111: #define DLL_ERROR4 "Error: Failed to load %s" Have you looked into all usages of this macro, also non-windows/shared code? Please make sure the replacement text makes sense in all those cases. ------------- PR: https://git.openjdk.org/jdk/pull/9749
