Re: [PATCH 3/3] selftests: Add printf attribute to ksefltest prints

From: Ilpo Järvinen
Date: Thu Aug 24 2023 - 09:11:34 EST


On Thu, 24 Aug 2023, Wieczor-Retman, Maciej wrote:

> Kselftest header defines multiple variadic function that use printf
> along with other logic
>
> There is no format checking for the variadic functions that use
> printing inside kselftest.h. Because of this the compiler won't
> be able to catch instances of mismatched print formats and debugging
> tests might be more difficult
>
> Add the common __printf attribute macro to kselftest.h
>
> Add __printf attribute to every function using formatted printing with
> variadic arguments

Please add . to terminate the sentences.

The patch looks fine:
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>

...However, there are formatting errors it found yet to fix.

--
i.

> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@xxxxxxxxx>
> ---
> tools/testing/selftests/kselftest.h | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/kselftest.h b/tools/testing/selftests/kselftest.h
> index 829be379545a..ff47ed711879 100644
> --- a/tools/testing/selftests/kselftest.h
> +++ b/tools/testing/selftests/kselftest.h
> @@ -77,6 +77,8 @@
> #define KSFT_XPASS 3
> #define KSFT_SKIP 4
>
> +#define __printf(a, b) __attribute__((format(printf, a, b)))
> +
> /* counters */
> struct ksft_count {
> unsigned int ksft_pass;
> @@ -134,7 +136,7 @@ static inline void ksft_print_cnts(void)
> ksft_cnt.ksft_xskip, ksft_cnt.ksft_error);
> }
>
> -static inline void ksft_print_msg(const char *msg, ...)
> +static inline __printf(1, 2) void ksft_print_msg(const char *msg, ...)
> {
> int saved_errno = errno;
> va_list args;
> @@ -146,7 +148,7 @@ static inline void ksft_print_msg(const char *msg, ...)
> va_end(args);
> }
>
> -static inline void ksft_test_result_pass(const char *msg, ...)
> +static inline __printf(1, 2) void ksft_test_result_pass(const char *msg, ...)
> {
> int saved_errno = errno;
> va_list args;
> @@ -160,7 +162,7 @@ static inline void ksft_test_result_pass(const char *msg, ...)
> va_end(args);
> }
>
> -static inline void ksft_test_result_fail(const char *msg, ...)
> +static inline __printf(1, 2) void ksft_test_result_fail(const char *msg, ...)
> {
> int saved_errno = errno;
> va_list args;
> @@ -186,7 +188,7 @@ static inline void ksft_test_result_fail(const char *msg, ...)
> ksft_test_result_fail(fmt, ##__VA_ARGS__);\
> } while (0)
>
> -static inline void ksft_test_result_xfail(const char *msg, ...)
> +static inline __printf(1, 2) void ksft_test_result_xfail(const char *msg, ...)
> {
> int saved_errno = errno;
> va_list args;
> @@ -200,7 +202,7 @@ static inline void ksft_test_result_xfail(const char *msg, ...)
> va_end(args);
> }
>
> -static inline void ksft_test_result_skip(const char *msg, ...)
> +static inline __printf(1, 2) void ksft_test_result_skip(const char *msg, ...)
> {
> int saved_errno = errno;
> va_list args;
> @@ -215,7 +217,7 @@ static inline void ksft_test_result_skip(const char *msg, ...)
> }
>
> /* TODO: how does "error" differ from "fail" or "skip"? */
> -static inline void ksft_test_result_error(const char *msg, ...)
> +static inline __printf(1, 2) void ksft_test_result_error(const char *msg, ...)
> {
> int saved_errno = errno;
> va_list args;
> @@ -262,7 +264,7 @@ static inline int ksft_exit_fail(void)
> ksft_cnt.ksft_xfail + \
> ksft_cnt.ksft_xskip)
>
> -static inline int ksft_exit_fail_msg(const char *msg, ...)
> +static inline __printf(1, 2) int ksft_exit_fail_msg(const char *msg, ...)
> {
> int saved_errno = errno;
> va_list args;
> @@ -289,7 +291,7 @@ static inline int ksft_exit_xpass(void)
> exit(KSFT_XPASS);
> }
>
> -static inline int ksft_exit_skip(const char *msg, ...)
> +static inline __printf(1, 2) int ksft_exit_skip(const char *msg, ...)
> {
> int saved_errno = errno;
> va_list args;
>