On Wed, 3 Apr 2024 08:49:01 GMT, Daniel JeliƄski <djelin...@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!
>
> src/jdk.crypto.cryptoki/unix/native/libj2pkcs11/p11_md.c line 94:
> 
>> 92:     char *systemErrorMessage;
>> 93:     char *exceptionMessage;
>> 94:     const char *getFunctionListStr = "C_GetFunctionList";
> 
> If this value ever gets used by ReleaseStringUTFChars, the failure will be 
> spectacular 🍿

We do have checks for jGetFunctionList != NULL before calling 
ReleaseStringUTFChars() with it. So, this shouldn't be an issue?

> src/jdk.crypto.cryptoki/unix/native/libj2pkcs11/p11_md.c line 165:
> 
>> 163:             if (ckAssertReturnValueOK(env, rv) == CK_ASSERT_OK) {
>> 164:                 goto setModuleData;
>> 165:             }
> 
> Do we need an `else goto cleanup` here?

Not really, the intention is to continue with the C_GetFunctionList (or the 
method named by "getFunctionListStr").

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18588#discussion_r1550237653
PR Review Comment: https://git.openjdk.org/jdk/pull/18588#discussion_r1550237417

Reply via email to