On Sat, 6 Aug 2022 11:36:44 GMT, Thomas Stuefe <[email protected]> wrote:
>> 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.
Agreed, will split this part into another RFE (initially didn't do this because
such a change felt too trivial for an entire entry in the JBS), but I do feel
like waiting for more opinions on what the prefix should be changed to before
doing that
-------------
PR: https://git.openjdk.org/jdk/pull/9749
