| Message ID | 20240501020813.264878-1-jhubbard@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/harness: fix many "format string is empty" warnings | expand |
Hi John, On Tue, Apr 30, 2024 at 07:08:13PM -0700, John Hubbard wrote: > In order to build with clang at all, in order to see these symptoms, one > must first apply Valentin Obst's build fix for LLVM [1]. Once that is > done, then when building with clang, via: > > make LLVM=1 -C tools/testing/selftests > > ...clang emits a "format string is empty" warning. (gcc also emits a > similar warning.) The warning you are describing here sounds like the same exact one that commit caed8eba2215 ("selftests: kselftest_harness: fix Clang warning about zero-length format") should have addressed in 6.9-rc5. Is this patch actually necessary? > Fix by passing NULL, instead of "", for the msg argument to Because this text is describing what was done in caed8eba2215... > ksft_test_result_code(). This removes dozens of warnings and a few > errors (some tests have -Werror set). > > [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/ > > Cc: Valentin Obst <kernel@valentinobst.de> > Cc: Kees Cook <keescook@chromium.org> > Cc: Nick Desaulniers <ndesaulniers@google.com> > Cc: Nathan Chancellor <nathan@kernel.org> > Cc: Shuah Khan <shuah@kernel.org> > Cc: Justin Stitt <justinstitt@google.com> > Cc: Bill Wendling <morbo@google.com> > Signed-off-by: John Hubbard <jhubbard@nvidia.com> > --- > tools/testing/selftests/kselftest_harness.h | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h > index d98702b6955d..456b8694e678 100644 > --- a/tools/testing/selftests/kselftest_harness.h > +++ b/tools/testing/selftests/kselftest_harness.h > @@ -1207,8 +1207,10 @@ void __run_test(struct __fixture_metadata *f, > else > diagnostic = "unknown"; > > - ksft_test_result_code(t->exit_code, test_name, > - diagnostic ? "%s" : NULL, diagnostic); > + if (diagnostic) > + ksft_test_result_code(t->exit_code, test_name, "%s", diagnostic); > + else > + ksft_test_result_code(t->exit_code, test_name, NULL); but this diff is not doing that because it is based on a tree that has caed8eba2215; instead, it appears to be a completely identical transformation? Cheers, Nathan > free(test_name); > } > > > base-commit: 18daea77cca626f590fb140fc11e3a43c5d41354 > prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27 > -- > 2.45.0 >
On 5/1/24 8:13 AM, Nathan Chancellor wrote: > Hi John, > > On Tue, Apr 30, 2024 at 07:08:13PM -0700, John Hubbard wrote: >> In order to build with clang at all, in order to see these symptoms, one >> must first apply Valentin Obst's build fix for LLVM [1]. Once that is >> done, then when building with clang, via: >> >> make LLVM=1 -C tools/testing/selftests >> >> ...clang emits a "format string is empty" warning. (gcc also emits a >> similar warning.) > > The warning you are describing here sounds like the same exact one that > commit caed8eba2215 ("selftests: kselftest_harness: fix Clang warning > about zero-length format") should have addressed in 6.9-rc5. Is this > patch actually necessary? Apparently not! > >> Fix by passing NULL, instead of "", for the msg argument to > > Because this text is describing what was done in caed8eba2215... Yes. I took a while between discovering the issue, and posting a fix, I think that's why. > >> ksft_test_result_code(). This removes dozens of warnings and a few >> errors (some tests have -Werror set). >> >> [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/ >> >> Cc: Valentin Obst <kernel@valentinobst.de> >> Cc: Kees Cook <keescook@chromium.org> >> Cc: Nick Desaulniers <ndesaulniers@google.com> >> Cc: Nathan Chancellor <nathan@kernel.org> >> Cc: Shuah Khan <shuah@kernel.org> >> Cc: Justin Stitt <justinstitt@google.com> >> Cc: Bill Wendling <morbo@google.com> >> Signed-off-by: John Hubbard <jhubbard@nvidia.com> >> --- >> tools/testing/selftests/kselftest_harness.h | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h >> index d98702b6955d..456b8694e678 100644 >> --- a/tools/testing/selftests/kselftest_harness.h >> +++ b/tools/testing/selftests/kselftest_harness.h >> @@ -1207,8 +1207,10 @@ void __run_test(struct __fixture_metadata *f, >> else >> diagnostic = "unknown"; >> >> - ksft_test_result_code(t->exit_code, test_name, >> - diagnostic ? "%s" : NULL, diagnostic); >> + if (diagnostic) >> + ksft_test_result_code(t->exit_code, test_name, "%s", diagnostic); >> + else >> + ksft_test_result_code(t->exit_code, test_name, NULL); > > but this diff is not doing that because it is based on a tree that has > caed8eba2215; instead, it appears to be a completely identical > transformation? > It's nice to see that someone else had the same fix, anyway. Sorry about the noise, and yes let's please just drop this one. thanks,
diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h index d98702b6955d..456b8694e678 100644 --- a/tools/testing/selftests/kselftest_harness.h +++ b/tools/testing/selftests/kselftest_harness.h @@ -1207,8 +1207,10 @@ void __run_test(struct __fixture_metadata *f, else diagnostic = "unknown"; - ksft_test_result_code(t->exit_code, test_name, - diagnostic ? "%s" : NULL, diagnostic); + if (diagnostic) + ksft_test_result_code(t->exit_code, test_name, "%s", diagnostic); + else + ksft_test_result_code(t->exit_code, test_name, NULL); free(test_name); }
In order to build with clang at all, in order to see these symptoms, one must first apply Valentin Obst's build fix for LLVM [1]. Once that is done, then when building with clang, via: make LLVM=1 -C tools/testing/selftests ...clang emits a "format string is empty" warning. (gcc also emits a similar warning.) Fix by passing NULL, instead of "", for the msg argument to ksft_test_result_code(). This removes dozens of warnings and a few errors (some tests have -Werror set). [1] https://lore.kernel.org/all/20240329-selftests-libmk-llvm-rfc-v1-1-2f9ed7d1c49f@valentinobst.de/ Cc: Valentin Obst <kernel@valentinobst.de> Cc: Kees Cook <keescook@chromium.org> Cc: Nick Desaulniers <ndesaulniers@google.com> Cc: Nathan Chancellor <nathan@kernel.org> Cc: Shuah Khan <shuah@kernel.org> Cc: Justin Stitt <justinstitt@google.com> Cc: Bill Wendling <morbo@google.com> Signed-off-by: John Hubbard <jhubbard@nvidia.com> --- tools/testing/selftests/kselftest_harness.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) base-commit: 18daea77cca626f590fb140fc11e3a43c5d41354 prerequisite-patch-id: b901ece2a5b78503e2fb5480f20e304d36a0ea27