On Wed, 3 Apr 2024 18:11:34 GMT, Valerie Peng <[email protected]> wrote:
>> 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?
this will be an issue if `jGetFunctionList != NULL` and `dlopen` fails,
resulting in `goto cleanup` before `getFunctionListStr` is reassigned.
>> 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").
if that's the intention, then you shouldn't be using `ckAssertReturnValueOK`,
which throws an exception if `rv` is anything other than `CK_ASSERT_OK`
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/18588#discussion_r1550245099
PR Review Comment: https://git.openjdk.org/jdk/pull/18588#discussion_r1550243300