fix: android jni unused-parameter #1772
Conversation
| @@ -35,6 +35,8 @@ add_subdirectory(../../../../.. build) | |||
|
|
|||
| add_library("avif_android" SHARED "libavif_jni.cc") | |||
|
|
|||
| target_compile_options(avif_android PRIVATE $<TARGET_PROPERTY:avif,COMPILE_OPTIONS>) | |||
There was a problem hiding this comment.
Vignesh: I didn't review the change to this file. The changes to libavif_jni.cc seem like a good idea.
There was a problem hiding this comment.
This change isn't actually necessary and I'd be fine with removing it. It makes it so that this warning causes errors on build, but that's not strictly necessary. If something like #1778 gets merged then this would be redundant.
|
Thank you, i will take a look at this PR. |
| @@ -19,10 +19,10 @@ | |||
| #define FUNC(RETURN_TYPE, NAME, ...) \ | |||
| extern "C" { \ | |||
| JNIEXPORT RETURN_TYPE Java_org_aomedia_avif_android_AvifDecoder_##NAME( \ | |||
| JNIEnv* env, jobject thiz, ##__VA_ARGS__); \ | |||
| __VA_ARGS__); \ | |||
There was a problem hiding this comment.
hmm, i don't like the idea of repeating the jni env parameters in all the functions. it pushes the actual argument list to the back and it's a mix of named and unnamed parameters.
how about this instead: keep this macro as is. add a new macro like so:
#define IGNORE_UNUSED_JNIPARAMETERS \
(void) env; \
(void) thiz
and add this as the first line to each of the functions? this way it is uniform how we handle it in all the functions (it doesn't really matter if we use these variables in these functinos or not since we know what we are ignoring here).
There was a problem hiding this comment.
It was between that approach and the one I have here, and I didn't have strong opinion either way. I'd be fine with adding an ignore parameter macro
fdintino
force-pushed
the
fix/libavif-jni-unused-param
branch
from
f56cf4e
to
3284e05
Compare
|
thank you for the PR! |
The more stringent error and warning compiler flags in the root CMakeLists.txt are set with
add_compile_optionsand so, as directory properties, they don't get applied to the avif_android target. If-Wall -Werroror/W4 /WXare enabled for avif_android, the compiler fails on-Werror,-Wunused-parameter. This issue is fixed by defining a macro,IGNORE_UNUSED_JNIPARAMETERS, that is called at the top of any functions that don't reference theenvorthizparameters.Note: this file appears to not have had clang-format run on it, so I didn't do any reformatting.