On Wed, 3 Apr 2024 18:11:34 GMT, Valerie Peng <valer...@openjdk.org> 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