On Sat, 6 Aug 2022 11:36:44 GMT, Thomas Stuefe <stu...@openjdk.org> 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

Reply via email to