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@xxxxxxxxxxxxxxx/
>
> Cc: Valentin Obst <kernel@xxxxxxxxxxxxxxx>
> Cc: Kees Cook <keescook@xxxxxxxxxxxx>
> Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> Cc: Nathan Chancellor <nathan@xxxxxxxxxx>
> Cc: Shuah Khan <shuah@xxxxxxxxxx>
> Cc: Justin Stitt <justinstitt@xxxxxxxxxx>
> Cc: Bill Wendling <morbo@xxxxxxxxxx>
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> ---
> 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
>
