Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Android.mk: add -Wno-unused-parameter CFLAGS for xtest #751

Merged
merged 1 commit into from
Jul 16, 2024

Conversation

makohoek
Copy link
Contributor

Commit bcd5583 ("xtest: add asymmetric cipher perf test") introduces a build error for xtest on AOSP 14:

external/optee_test/host/xtest/asym_perf.c:125:71: error: unused parameter 'main_algo' [-Werror,-Wunused-parameter] static void usage(const char *progname, uint32_t width_bits, uint32_t main_algo,
                                                                      ^
external/optee_test/host/xtest/asym_perf.c:126:14: error: unused parameter 'mode' [-Werror,-Wunused-parameter]
                  uint32_t mode, uint32_t salt_len, uint32_t size,
                           ^
external/optee_test/host/xtest/asym_perf.c:127:14: error: unused parameter 'crypto_algo' [-Werror,-Wunused-parameter]
                  uint32_t crypto_algo, int warmup, uint32_t l, uint32_t n)
                           ^
external/optee_test/host/xtest/asym_perf.c:668:76: error: unused parameter 'size' [-Werror,-Wunused-parameter] static int check_rsa_hash_params(uint32_t crypto_algo, int width_bits, int size,
                                                                           ^
4 errors generated.
13:15:41 ninja failed with: exit status 1

Adding -Wno-unused-parameter to the build flags seems a reasonable fix since this is a test executable.

Add it to fix the build error.

@jenswi-linaro
Copy link
Contributor

How about removing the -Werror switch instead?

@makohoek
Copy link
Contributor Author

Hi @jenswi-linaro ,

Thank you for your review.

We can't (and should not) drop the -Werror switch because it's a flag that's globally enforced by the Android build system since 2017:
https://android-review.googlesource.com/c/platform/build/+/529795
It's possible to whitelist projects using ANDROID_WARNING_ALLOWED_PROJECTS but that's not common knowledge.

To me, it was best to disable this warning in the project's Android.mk file.

We could use -Wno-error otherwise, but that seems more permissive than what's actually needed.

Should I add a note in the commit message explaining why we can't remove -Werror ?

@jenswi-linaro
Copy link
Contributor

Another option is to fix the problem with the warnings.

We normally don't use -Werror to avoid unexpected errors due to some odd or new toolchain finding something new to warn about.

In CI we use -Werror since this is the one place where warnings shouldn't be accepted. I guess we're missing a -Werror somewhere since you found this with AOSP.

@makohoek
Copy link
Contributor Author

I can fix the warnings as well. I did not want to touch the original code that much since I'm mostly a integrating/distributing xtest without knowing too much about the underlying codebase.

However, I've quickly looked at the CI and I don't know what is missing.

I'll submit a new version that fixes all warnings on Android when building with -Werror -Wall.

@jenswi-linaro
Copy link
Contributor

Thanks.

@jenswi-linaro
Copy link
Contributor

Looking at CMakeLists.txt, I see that we are using -Wno-unused-parameter. So your patch makes sense.
Please apply:
Reviewed-by: Jens Wiklander <[email protected]>

Commit bcd5583 ("xtest: add asymmetric cipher perf test")
introduces a build error for xtest on AOSP 14:

external/optee_test/host/xtest/asym_perf.c:125:71: error: unused parameter 'main_algo' [-Werror,-Wunused-parameter]
static void usage(const char *progname, uint32_t width_bits, uint32_t main_algo,
                                                                      ^
external/optee_test/host/xtest/asym_perf.c:126:14: error: unused parameter 'mode' [-Werror,-Wunused-parameter]
                  uint32_t mode, uint32_t salt_len, uint32_t size,
                           ^
external/optee_test/host/xtest/asym_perf.c:127:14: error: unused parameter 'crypto_algo' [-Werror,-Wunused-parameter]
                  uint32_t crypto_algo, int warmup, uint32_t l, uint32_t n)
                           ^
external/optee_test/host/xtest/asym_perf.c:668:76: error: unused parameter 'size' [-Werror,-Wunused-parameter]
static int check_rsa_hash_params(uint32_t crypto_algo, int width_bits, int size,
                                                                           ^
4 errors generated.
13:15:41 ninja failed with: exit status 1

Adding -Wno-unused-parameter to the build flags seems a reasonable fix
since this is a test executable.

Add it to fix the build error.

Signed-off-by: Mattijs Korpershoek <[email protected]>
Reviewed-by: Jens Wiklander <[email protected]>
@makohoek
Copy link
Contributor Author

Thanks, added the Reviewed-by.

@jforissier jforissier merged commit cec6f47 into OP-TEE:master Jul 16, 2024
1 check passed
@makohoek makohoek deleted the android-unused-param branch July 17, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants