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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a93a4f17-73c7-cf6a-e44f-78ece1e25e93@linux.intel.com>
Date:   Thu, 24 Aug 2023 16:10:12 +0300 (EEST)
From:   Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To:     "Wieczor-Retman, Maciej" <maciej.wieczor-retman@...el.com>
cc:     LKML <linux-kernel@...r.kernel.org>,
        Reinette Chatre <reinette.chatre@...el.com>,
        fenghua.yu@...el.com
Subject: Re: [PATCH 3/3] selftests: Add printf attribute to ksefltest
 prints

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@...ux.intel.com>

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

-- 
 i.

> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@...el.com>
> ---
>  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;
>