On Sat, 6 Aug 2022 11:14:55 GMT, Julian Waters <jwat...@openjdk.org> wrote:
>> 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.
>
> The only non Windows code that uses the `DLL_ERROR4` macro is Unix's
> java_md_common.c, where it reports the same `JVM_FindClassFromBootLoader`
> issue that Windows also does, and the shared args.c when a file cannot be
> found by path:
>
> static JLI_List expandArgFile(const char *arg) {
> JLI_List rv;
> struct stat st;
> FILE *fptr = fopen(arg, "r");
>
> /* arg file cannot be opened */
> if (fptr == NULL || fstat(fileno(fptr), &st) != 0) {
> JLI_ReportMessage(CFG_ERROR6, arg);
> exit(1);
> } else {
> if (st.st_size > MAX_ARGF_SIZE) {
> JLI_ReportMessage(CFG_ERROR10, MAX_ARGF_SIZE);
> exit(1);
> }
> }
>
> rv = readArgFile(fptr);
>
> /* error occurred reading the file */
> if (rv == NULL) {
> JLI_ReportMessage(DLL_ERROR4, arg);
> exit(1);
> }
> fclose(fptr);
>
> return rv;
> }
>
>
> In any case, the only thing that would change would be "Error: loading:" to
> "Error: Failed to load" as the prefix of the message, which semantically is
> the same whatever the context may be, just a small change I thought that
> would look better whenever the macro is used
You also have one use where the VM does a GetProcAddress on windows. "Failed to
load" feels wrong for a failed dlsym. But "Error loading" is not much better
either, I admit.
In general, the smaller and focused you keep changes, the easier they are to
review and the better they are down-portable if that should be needed. I
usually try to do aesthetic changes in separate RFEs. In this change, if you
want to fix the display of additional diagnostic info, I'd just do that.
-------------
PR: https://git.openjdk.org/jdk/pull/9749
