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

David Holmes dholmes at openjdk.org
Tue Apr 2 02:02:02 UTC 2024


On Fri, 22 Mar 2024 07:25:32 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> Hi, please consider this additional logging.
>> 
>> Thanks, Robbin
>
> Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   windows logging

Changes requested by dholmes (Reviewer).

src/hotspot/os/windows/os_windows.cpp line 1289:

> 1287:     buf[end] = '\0';
> 1288:   }
> 1289:   return end;

This should have copied the explanatory comment as well. 

Pre-existing, but is  "./r/n" always added? If so the 'if' checks look weird - why would you not just set `buf[end-3] = '\0'` directly? If the formatting is actually variable this code will remove ".", "/r", "/n", "./r", "./n", "/r/n" and "./r/n". ???

src/hotspot/os/windows/os_windows.cpp line 1295:

> 1293:   void* ret = ::GetProcAddress((HMODULE)lib, name);
> 1294:   DWORD errval;
> 1295:   if ((errval = GetLastError()) != 0) {

`GetProcAddress` doesn't clear last-error on success, so you should only be calling `GetLastError` if the return value was null.

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

PR Review: https://git.openjdk.org/jdk/pull/18404#pullrequestreview-1972471501
PR Review Comment: https://git.openjdk.org/jdk/pull/18404#discussion_r1547043146
PR Review Comment: https://git.openjdk.org/jdk/pull/18404#discussion_r1547041869


More information about the hotspot-runtime-dev mailing list