On Tue, 22 Apr 2025 23:29:02 GMT, Jiangli Zhou <jian...@openjdk.org> wrote:

>> Please review this PR that changes to use `NativeLibraries.loadLibrary()` 
>> for loading the `libsyslookup` in `jdk.internal.foreign.SystemLookup` class.
>> 
>> `NativeLibraries.loadLibrary()` handles both the shared library and (static) 
>> built-in library loading properly. On `static-jdk`, calling 
>> `NativeLibraries.loadLibrary()` for `systlookup` library can find the 
>> built-in library by looking up using `JNI_OnLoad_syslookup`. The current 
>> change adds `DEF_STATIC_JNI_OnLoad` in syslookup.c (in shared, windows and 
>> aix versions) for that purpose.
>> 
>> In addition to GHA testing, I tested the change on static-jdk with jdk tier1 
>> tests on linux-x64 locally. All java/foreign/* jdk tier1 tests pass with the 
>> change. Without the change, there are about 60 java/foreign/* jdk tier1 
>> tests fail on static-jdk.
>
> Jiangli Zhou has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Update src/java.base/share/classes/jdk/internal/foreign/SystemLookup.java
>   
>   Thanks.
>   
>   I used `jdk.internal.loader.NativeLibraries` as a quick change before 
> adding adding import. I neglected to to clean up that part.
>   
>   Co-authored-by: Chen Liang <li...@openjdk.org>

src/java.base/share/native/libsyslookup/syslookup.c line 30:

> 28: #include <stdlib.h>
> 29: 
> 30: #include <jni.h>

Do we need this include? DEF_STATIC_JNI_OnLoad is defined by jni_util.h, which 
should include jni.h?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24801#discussion_r2055074496

Reply via email to