On Thu, 4 Apr 2024 21:23:25 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> This PR fixes a problem regarding the usage of dlerror() where an earlier
>> error message causes a premature error out. Added extra code to clear out
>> earlier error message and made minor code refactoring.
>>
>> No regression test as this can't be reproduced using the NSS library from
>> artifactory and thus the noreg-hard label.
>>
>> Thanks!
>
> 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?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18588#discussion_r1556191787
