Alternative to reinterpret_cast for JNI?

user_6449273 0
Viewed 778

I am following the Google tips to implement a JNI layer between my Android app and my C++ library. It suggests to use the following code to register native methods when the library is loaded:

JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) {
    JNIEnv* env;
    if (vm->GetEnv(reinterpret_cast<void**>(&env), JNI_VERSION_1_6) != JNI_OK) {
        return JNI_ERR;
    }

    ...

    // Register your class' native methods.
    static const JNINativeMethod methods[] = {
        {"nativeFoo", "()V", reinterpret_cast<void*>(nativeFoo)},
        {"nativeBar", "(Ljava/lang/String;I)Z", reinterpret_cast<void*>(nativeBar)},
    };
    int rc = env->RegisterNatives(c, methods, sizeof(methods)/sizeof(JNINativeMethod));
    ...
}

I am quite new to C++ so I decided to use clang-tidy to ensure my C++ code is modern and safe. clang-tidy reports:

error: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast,-warnings-as-errors]

According to the clang-tidy documentation:

cppcoreguidelines-pro-type-reinterpret-cast

This check flags all uses of reinterpret_cast in C++ code.

Use of these casts can violate type safety and cause the program to access a variable that is actually of type X to be accessed as if it were of an unrelated type Z.

This rule is part of the “Type safety” profile of the C++ Core Guidelines, see https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Pro-type-reinterpretcast.

So I have a few options:

  1. Disable this check and risk using reinterpret_cast inappropriately elsewhere
  2. Ignore the check everywhere where I need to use it and create a messy codebase
  3. Find some alternative way of implementing this more safely

I would like to do 3 if it's possible but I'm not quite sure where to start.

java-native-interface android c++
1 Answers

It’s not clear why GetEnv wants a void** rather than a JNIEnv** (what other type would you use?), but you can avoid the reinterpret_cast and the concomitant undefined behavior(!) with a temporary variable:

void *venv;
if (vm->GetEnv(&venv, JNI_VERSION_1_6) != JNI_OK) {
    return JNI_ERR;
}
const auto env=static_cast<JNIEnv*>(venv);

Note that a static_cast from void* is every bit as dangerous as a reinterpret_cast, but its use here is correct, unavoidable, and shouldn’t produce linter warnings.

In the function pointer case there’s nothing to be done: reinterpret_cast is the only, correct choice (and for void* rather than some placeholder function pointer type like void (*)() its correctness is not guaranteed by C++, although it works widely and POSIX does guarantee it). You can of course hide that conversion in a function (template) so that the linter can be told to ignore only it, but make sure to use a clear name to avoid “hiding” the conversion.

user_85862270
Related