On Thu, 18 Aug 2022 09:46:56 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> Let's back up a bit. >> >> I looked again, and think @dholmes-ora was originally right when he wrote >> that this coding does not get called on Windows. Not sure why I thought >> differently, but I cannot find a single callsite on windows. >> >> But that is weird, since originally there was an error reported on Windows: >> https://bugs.openjdk.org/browse/JDK-8291917 reported a real problem about >> windows printing useless error messages. What was the original problem again? >> >> If we are right and this code is not needed, I'd just scrap this function on >> Windows altogether. > > 8291917 was simply about making error messages more informative on Windows > when loading the C Runtime or Java Virtual Machine dlls failed (Right now it > just prints an unhelpful "Error: loading: ...") - It consequently depended on > this RFE for the functionality. Although it _can_ use the other error > reporting utilities the JDK has elsewhere, this one happens to be the most > convenient, but a more important reason is that the other utilities provided > by the JDK all also use the same flawed logic as this one, if we can decide > how to fix this function here and now, we'd be able to extrapolate the > solution to those other areas as well with very little fuss, so I think > fixing this would be better than simply opting to remove it Okay, this is for future use. We don't usually add complexity as a preparation for the future. We usually include usage and functionality in one RFE. That way reviewers can see how the feature is supposed to be used, and whether its worth to keep it. You may hear other opinions, but what I would do: - 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. - If that turns out to be many call sites, we can think about adding a windows specific JLI_ReportErrorMessageSysWin32. - If that's just 2-5 sites or so, leave it and just scrap JLI_ReportErrorMessageSys for windows. Just my 5 cent. ------------- PR: https://git.openjdk.org/jdk/pull/9870
