On Mon, 8 Apr 2024 17:38:51 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Valerie Peng has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Address one more review comment.
>
> src/jdk.crypto.cryptoki/unix/native/libj2pkcs11/p11_md.c line 188:
>
>> 186: TRACE1("Connect: No %s func\n", getFunctionListStr);
>> 187: p11ThrowIOException(env, "ERROR: C_GetFunctionList == NULL");
>> 188: goto cleanup;
>
> Just a small comment. The `dlerror` is only for debugging purpose but the
> code above looks like it is used to determine whether to `goto cleanup`.
>
> How about this:
>
> if (C_GetFunctionList == NULL) {
> if ((systemErrorMessage = dlerror()) != NULL){
> p11ThrowIOException(env, systemErrorMessage);
> } else {
> TRACE1("Connect: No %s func\n", getFunctionListStr);
> p11ThrowIOException(env, "ERROR: C_GetFunctionList == NULL");
> }
> goto cleanup;
> }
>
> Also, why is TRACE1 not called when `dlerror` is not NULL?
Sure, thanks for the suggestion. I'll change and re-run the tests.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18588#discussion_r1556399726
