On Thu, 18 Aug 2022 15:46:04 GMT, Julian Waters <jwat...@openjdk.org> wrote:
> Initially I did intend to do that, but a raw GetLastError wouldn't exactly be > very helpful when a failure does occur I disagree. Error codes are well documented, unlike posix errno with defined numerical values, and often more helpful than the localized error text. >, and unfortunately formatting the proper message from it takes quite a bit of >ceremony (Thanks for nothing Win32 API!) Then factor that out into an utility function similar to strerror and use that as input to JLI_ReportErrorMessage. But again, only if it is worth the complexity. As long as there is not a single use case inside libjli we should not add this function. >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. Yes. And that's fine. Please deal with one problem at a time, libjli in this case, and do a clearly defined solution that fits the problem in scope and complexity. Please follow the advice of reviewers, because you burn valuable reviewer time. > I did hope that a warning or outright refusal to concat the errors when > either GetLastError and errno != 0 was a good solution, though I disagree. It is more confusing and not worth the added complexity. Look at it that way, at one hand you want to resolve errorcode and think raw errcode values are too much for the lay analyst, on the other hand you want to show him two potentially completely unrelated error texts, together with a cryptic warning, and let him figure out what that means? Let's do this simple and right. So far I have not seen a single use of this function inside the libjli codebase. So come up with that first as I wrote before. Then, if you see multiple use cases, we can maybe think of unifying code into a helper function. If there are no usages, just remove the function. ------------- PR: https://git.openjdk.org/jdk/pull/9870
