zeroshade commented on code in PR #4423:
URL: https://github.com/apache/arrow-adbc/pull/4423#discussion_r3461000991
##########
java/driver/jni/src/main/cpp/jni_wrapper.cc:
##########
@@ -216,51 +216,47 @@ jmethodID RequireMethod(JNIEnv* env, jclass klass,
std::string_view name,
return method;
}
-struct JniStringView {
- JNIEnv* env;
- jstring jni_string;
- const char* value;
-
- explicit JniStringView(JNIEnv* env, jstring jni_string)
- : env(env), jni_string(jni_string), value(nullptr) {
- if (jni_string == nullptr) {
- throw AdbcException{ADBC_STATUS_INTERNAL, "Java string was nullptr"};
- }
- value = env->GetStringUTFChars(jni_string, nullptr);
- if (value == nullptr) {
- throw AdbcException{ADBC_STATUS_INTERNAL,
- "Java string was nullptr (could not get string
contents)"};
- }
+std::string GetJniUtf8String(JNIEnv* env, jbyteArray jni_string) {
+ if (jni_string == nullptr) {
+ throw AdbcException{ADBC_STATUS_INTERNAL, "Java byte array was nullptr"};
}
- ~JniStringView() {
- if (jni_string == nullptr) {
- return;
- }
-
- env->ReleaseStringUTFChars(jni_string, value);
- env->DeleteLocalRef(jni_string);
- jni_string = nullptr;
+ jsize length = env->GetArrayLength(jni_string);
+ if (env->ExceptionCheck()) {
+ throw AdbcException{ADBC_STATUS_INTERNAL, "Could not get byte array
length"};
}
-};
-std::string GetJniString(JNIEnv* env, jstring jni_string) {
- JniStringView view(env, jni_string);
- return std::string(view.value);
+ std::string result(static_cast<size_t>(length), '\0');
+ if (length > 0) {
+ env->GetByteArrayRegion(jni_string, 0, length,
+ reinterpret_cast<jbyte*>(result.data()));
+ if (env->ExceptionCheck()) {
+ throw AdbcException{ADBC_STATUS_INTERNAL, "Could not get byte array
contents"};
+ }
+ }
+ return result;
}
-std::optional<std::string> MaybeGetJniString(JNIEnv* env, jstring jni_string) {
+std::optional<std::string> MaybeGetJniUtf8String(JNIEnv* env, jbyteArray
jni_string) {
if (jni_string == nullptr) {
return std::nullopt;
}
- JniStringView view(env, jni_string);
- return std::string(view.value);
+ return GetJniUtf8String(env, jni_string);
}
-template <typename Callable>
-auto WithJniString(JNIEnv* env, jstring jni_string, Callable&& callable) {
- JniStringView view(env, jni_string);
- return callable(view.value);
+jbyteArray MakeJniUtf8String(JNIEnv* env, const char* value, size_t length) {
Review Comment:
jstring isn't utf8 and can't be utf8? Just wondering why we're using
`jbyteArray` instead of `jstring` for utf8 strings. Why do we need to have the
intermediate `jbyteArray` and then call `GetJniUtf8String` instead of having
this go straight to it? (there's probably some jni thing I'm not aware of which
is the reason)
--
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]
