Copilot commented on code in PR #4440:
URL: https://github.com/apache/arrow-adbc/pull/4440#discussion_r3464254742
##########
java/driver/jni/src/main/cpp/jni_wrapper.cc:
##########
@@ -1562,4 +1562,27 @@
Java_org_apache_arrow_adbc_driver_jni_impl_NativeAdbc_databaseSetOptionString(
e.ThrowJavaException(env);
}
}
+
+// wrapper around GetJniByteBuffer for direct unit testing
+JNIEXPORT jbyteArray JNICALL
+Java_org_apache_arrow_adbc_driver_jni_impl_NativeAdbc_internalGetByteBuffer(
+ JNIEnv* env, [[maybe_unused]] jclass self, jobject input) {
+ std::vector<uint8_t> scratch;
+ size_t length = 0;
+ try {
+ const uint8_t* raw = GetJniByteBuffer(env, input, scratch, length);
+ jbyteArray result = env->NewByteArray(static_cast<jsize>(length));
+ if (result == nullptr || env->ExceptionCheck()) return nullptr;
+ if (length > 0) {
+ env->SetByteArrayRegion(result, 0, static_cast<jsize>(length),
+ reinterpret_cast<const jbyte*>(raw));
+ if (env->ExceptionCheck()) return nullptr;
+ }
+ return result;
+ } catch (const AdbcException& e) {
+ e.ThrowJavaException(env);
+ return nullptr;
+ }
+ return nullptr;
Review Comment:
`internalGetByteBuffer` doesn’t check whether `GetJniByteBuffer` failed
(returned null and/or left a pending Java exception) before using
`length`/`raw`. Other call sites (e.g. `statementSetSubstraitPlan`) guard
against this. Without the guard, it’s possible to call `SetByteArrayRegion`
with a null pointer if `length` was set before the failure, and it also does
extra JNI work while an exception is pending.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
