RFR: 8328630: Add logging when needed symbols in dll are missing. [v3]

[David Holmes]

On Tue, 2 Apr 2024 07:01:43 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> That said, why are you duplicating the logic from `os::lasterror` instead of just passing it the buf to fill in?
>> 
>> if (ret == nullptr) {
>>   char buf[512];
>>   os::lasterror(buf, sizeof(buf));
>>   log_debug(os)(...);
>> }
>
>> `GetProcAddress` doesn't clear last-error on success, so you should only be calling `GetLastError` if the return value was null.
> 
> It seem like you are correct. I didn't find anything on GetProcAddress manpage, but on GetLastError page it says: "If the function is not documented to set the last-error code, the value returned by this function is simply the most recent last-error code"
> 
> I'll update.
> 
>> That said, why are you duplicating the logic from `os::lasterror` instead of just passing it the buf to fill in?
>> 
>> ```
>> if (ret == nullptr) {
>>   char buf[512];
>>   os::lasterror(buf, sizeof(buf));
>>   log_debug(os)(...);
>> }
>> ```
> 
> It's not clear to me that GetProcAddress will not return NULL without setting GetLastError.
> Thus calling os::lasterror could accidentally return error string for errno instead.
> Maybe this worry have no basis, what you think?

It should be easy to test what happens when you ask for a symbol known to not exist.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18404#discussion_r1547727605