[PATCH 0/6] Add printf attribute to kselftest functions

[PATCH 0/6] Add printf attribute to kselftest functions

[Wieczor-Retman, Maciej]

Kselftest.h declares many variadic functions that can print some
formatted message while also executing selftest logic. These
declarations don't have any compiler mechanism to verify if passed
arguments are valid in comparison with format specifiers used in
printf() calls.

Attribute addition can make debugging easier, the code more consistent
and prevent mismatched or missing variables.

Add a __printf() macro that validates types of variables passed to the
format string. The macro is similiarly used in other tools in the kernel.

Add __printf() attributes to function definitions inside kselftest.h that
use printing.

Adding the __printf() macro exposes some mismatches in format strings
across different selftests.

Fix the mismatched format specifiers in multiple tests.

Wieczor-Retman, Maciej (6):
  selftests: Add printf attribute to ksefltest prints
  selftests/cachestat: Fix print_cachestat format
  selftests/openat2: Fix wrong format specifier
  selftests/pidfd: Fix ksft print formats
  selftests/sigaltstack: Fix wrong format specifier
  selftests/kvm: Replace attribute with macro

 .../selftests/cachestat/test_cachestat.c       |  2 +-
 tools/testing/selftests/kselftest.h            | 18 ++++++++++--------
 .../testing/selftests/kvm/include/test_util.h  |  2 +-
 tools/testing/selftests/openat2/openat2_test.c |  2 +-
 .../selftests/pidfd/pidfd_fdinfo_test.c        |  2 +-
 tools/testing/selftests/pidfd/pidfd_test.c     | 12 ++++++------
 tools/testing/selftests/sigaltstack/sas.c      |  2 +-
 7 files changed, 21 insertions(+), 19 deletions(-)


base-commit: 13eb52f6293dbda02890698d92f3d9913d8d5aeb
-- 
2.42.0

[PATCH 1/6] selftests: Add printf attribute to ksefltest prints

[Wieczor-Retman, Maciej]

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.

Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.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;
-- 
2.42.0

[PATCH 2/6] selftests/cachestat: Fix print_cachestat format

[Wieczor-Retman, Maciej]

The format specifier in printf() call expects long int variables and
received long long int.

Change format specifiers to long long int so they match passed
variables.

Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
---
 tools/testing/selftests/cachestat/test_cachestat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
index 77620e7ecf56..a7adf42afb20 100644
--- a/tools/testing/selftests/cachestat/test_cachestat.c
+++ b/tools/testing/selftests/cachestat/test_cachestat.c
@@ -25,7 +25,7 @@ static const char * const dev_files[] = {
 void print_cachestat(struct cachestat *cs)
 {
 	ksft_print_msg(
-	"Using cachestat: Cached: %lu, Dirty: %lu, Writeback: %lu, Evicted: %lu, Recently Evicted: %lu\n",
+	"Using cachestat: Cached: %llu, Dirty: %llu, Writeback: %llu, Evicted: %llu, Recently Evicted: %llu\n",
 	cs->nr_cache, cs->nr_dirty, cs->nr_writeback,
 	cs->nr_evicted, cs->nr_recently_evicted);
 }
-- 
2.42.0

[PATCH 3/6] selftests/openat2: Fix wrong format specifier

[Wieczor-Retman, Maciej]

Ksft_print_msg() inside test_openat2_flags() uses the wrong format
specifier for printing test.how->flags variable.

Change the format specifier to %llX so it matches the printed variable.

Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
---
 tools/testing/selftests/openat2/openat2_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c
index 7fb902099de4..9024754530b2 100644
--- a/tools/testing/selftests/openat2/openat2_test.c
+++ b/tools/testing/selftests/openat2/openat2_test.c
@@ -300,7 +300,7 @@ void test_openat2_flags(void)
 
 			ksft_print_msg("openat2 unexpectedly returned ");
 			if (fdpath)
-				ksft_print_msg("%d['%s'] with %X (!= %X)\n",
+				ksft_print_msg("%d['%s'] with %X (!= %llX)\n",
 					       fd, fdpath, fdflags,
 					       test->how.flags);
 			else
-- 
2.42.0

[PATCH 4/6] selftests/pidfd: Fix ksft print formats

[Wieczor-Retman, Maciej]

Many calls to ksft print functions have format strings that don't match
with other passed arguments. One call expects a string but doesn't
provide any argument after the format string.

Fix format specifiers so they match the passed variables.

Add a missing variable to ksft_test_result_pass() inside
pidfd_fdinfo_test() so it matches other cases in the switch statement.

Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
---
 tools/testing/selftests/pidfd/pidfd_fdinfo_test.c |  2 +-
 tools/testing/selftests/pidfd/pidfd_test.c        | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
index 4e86f927880c..01cc37bf611c 100644
--- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
+++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
@@ -62,7 +62,7 @@ static void error_report(struct error *err, const char *test_name)
 		break;
 
 	case PIDFD_PASS:
-		ksft_test_result_pass("%s test: Passed\n");
+		ksft_test_result_pass("%s test: Passed\n", test_name);
 		break;
 
 	default:
diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c
index 00a07e7c571c..c081ae91313a 100644
--- a/tools/testing/selftests/pidfd/pidfd_test.c
+++ b/tools/testing/selftests/pidfd/pidfd_test.c
@@ -381,13 +381,13 @@ static int test_pidfd_send_signal_syscall_support(void)
 
 static void *test_pidfd_poll_exec_thread(void *priv)
 {
-	ksft_print_msg("Child Thread: starting. pid %d tid %d ; and sleeping\n",
+	ksft_print_msg("Child Thread: starting. pid %d tid %ld ; and sleeping\n",
 			getpid(), syscall(SYS_gettid));
 	ksft_print_msg("Child Thread: doing exec of sleep\n");
 
 	execl("/bin/sleep", "sleep", str(CHILD_THREAD_MIN_WAIT), (char *)NULL);
 
-	ksft_print_msg("Child Thread: DONE. pid %d tid %d\n",
+	ksft_print_msg("Child Thread: DONE. pid %d tid %ld\n",
 			getpid(), syscall(SYS_gettid));
 	return NULL;
 }
@@ -427,7 +427,7 @@ static int child_poll_exec_test(void *args)
 {
 	pthread_t t1;
 
-	ksft_print_msg("Child (pidfd): starting. pid %d tid %d\n", getpid(),
+	ksft_print_msg("Child (pidfd): starting. pid %d tid %ld\n", getpid(),
 			syscall(SYS_gettid));
 	pthread_create(&t1, NULL, test_pidfd_poll_exec_thread, NULL);
 	/*
@@ -480,10 +480,10 @@ static void test_pidfd_poll_exec(int use_waitpid)
 
 static void *test_pidfd_poll_leader_exit_thread(void *priv)
 {
-	ksft_print_msg("Child Thread: starting. pid %d tid %d ; and sleeping\n",
+	ksft_print_msg("Child Thread: starting. pid %d tid %ld ; and sleeping\n",
 			getpid(), syscall(SYS_gettid));
 	sleep(CHILD_THREAD_MIN_WAIT);
-	ksft_print_msg("Child Thread: DONE. pid %d tid %d\n", getpid(), syscall(SYS_gettid));
+	ksft_print_msg("Child Thread: DONE. pid %d tid %ld\n", getpid(), syscall(SYS_gettid));
 	return NULL;
 }
 
@@ -492,7 +492,7 @@ static int child_poll_leader_exit_test(void *args)
 {
 	pthread_t t1, t2;
 
-	ksft_print_msg("Child: starting. pid %d tid %d\n", getpid(), syscall(SYS_gettid));
+	ksft_print_msg("Child: starting. pid %d tid %ld\n", getpid(), syscall(SYS_gettid));
 	pthread_create(&t1, NULL, test_pidfd_poll_leader_exit_thread, NULL);
 	pthread_create(&t2, NULL, test_pidfd_poll_leader_exit_thread, NULL);
 
-- 
2.42.0

[PATCH 5/6] selftests/sigaltstack: Fix wrong format specifier

[Wieczor-Retman, Maciej]

The format specifier inside ksft printing function expects a long
unsigned int but the passed variable is of unsigned int type.

Fix the format specifier so it matches the passed variable.

Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
---
 tools/testing/selftests/sigaltstack/sas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
index 98d37cb744fb..07227fab1cc9 100644
--- a/tools/testing/selftests/sigaltstack/sas.c
+++ b/tools/testing/selftests/sigaltstack/sas.c
@@ -111,7 +111,7 @@ int main(void)
 
 	/* Make sure more than the required minimum. */
 	stack_size = getauxval(AT_MINSIGSTKSZ) + SIGSTKSZ;
-	ksft_print_msg("[NOTE]\tthe stack size is %lu\n", stack_size);
+	ksft_print_msg("[NOTE]\tthe stack size is %u\n", stack_size);
 
 	ksft_print_header();
 	ksft_set_plan(3);
-- 
2.42.0

[PATCH 6/6] selftests/kvm: Replace attribute with macro

[Wieczor-Retman, Maciej]

The __printf() macro is used in many tools in the linux kernel to
validate the format specifiers in functions that use printf. Some
selftests use it without putting it in a macro definition and some tests
import the kselftests.h header.

Use __printf() attribute instead of the full attribute since the macro
is inside kselftests.h and the header is already imported.

Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
---
 tools/testing/selftests/kvm/include/test_util.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index a6e9f215ce70..710a8a78e8ce 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -33,7 +33,7 @@ static inline int _no_printf(const char *format, ...) { return 0; }
 #define pr_info(...) _no_printf(__VA_ARGS__)
 #endif
 
-void print_skip(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
+void __printf(1, 2) print_skip(const char *fmt, ...);
 #define __TEST_REQUIRE(f, fmt, ...)				\
 do {								\
 	if (!(f))						\
-- 
2.42.0

Re: [PATCH 4/6] selftests/pidfd: Fix ksft print formats

[Ilpo Järvinen]

On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:

> Many calls to ksft print functions have format strings that don't match
> with other passed arguments. One call expects a string but doesn't
> provide any argument after the format string.
> 
> Fix format specifiers so they match the passed variables.
> 
> Add a missing variable to ksft_test_result_pass() inside
> pidfd_fdinfo_test() so it matches other cases in the switch statement.
> 
> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
> ---
>  tools/testing/selftests/pidfd/pidfd_fdinfo_test.c |  2 +-
>  tools/testing/selftests/pidfd/pidfd_test.c        | 12 ++++++------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
> index 4e86f927880c..01cc37bf611c 100644
> --- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
> +++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
> @@ -62,7 +62,7 @@ static void error_report(struct error *err, const char *test_name)
>  		break;
>  
>  	case PIDFD_PASS:
> -		ksft_test_result_pass("%s test: Passed\n");
> +		ksft_test_result_pass("%s test: Passed\n", test_name);

Missing test_name leads to crash so this looks a Fixes: class thing for
me but you didn't provide one.

>  		break;
>  
>  	default:
> diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c
> index 00a07e7c571c..c081ae91313a 100644
> --- a/tools/testing/selftests/pidfd/pidfd_test.c
> +++ b/tools/testing/selftests/pidfd/pidfd_test.c
> @@ -381,13 +381,13 @@ static int test_pidfd_send_signal_syscall_support(void)
>  
>  static void *test_pidfd_poll_exec_thread(void *priv)
>  {
> -	ksft_print_msg("Child Thread: starting. pid %d tid %d ; and sleeping\n",
> +	ksft_print_msg("Child Thread: starting. pid %d tid %ld ; and sleeping\n",
>  			getpid(), syscall(SYS_gettid));
>  	ksft_print_msg("Child Thread: doing exec of sleep\n");
>  
>  	execl("/bin/sleep", "sleep", str(CHILD_THREAD_MIN_WAIT), (char *)NULL);
>  
> -	ksft_print_msg("Child Thread: DONE. pid %d tid %d\n",
> +	ksft_print_msg("Child Thread: DONE. pid %d tid %ld\n",
>  			getpid(), syscall(SYS_gettid));
>  	return NULL;
>  }
> @@ -427,7 +427,7 @@ static int child_poll_exec_test(void *args)
>  {
>  	pthread_t t1;
>  
> -	ksft_print_msg("Child (pidfd): starting. pid %d tid %d\n", getpid(),
> +	ksft_print_msg("Child (pidfd): starting. pid %d tid %ld\n", getpid(),
>  			syscall(SYS_gettid));
>  	pthread_create(&t1, NULL, test_pidfd_poll_exec_thread, NULL);
>  	/*
> @@ -480,10 +480,10 @@ static void test_pidfd_poll_exec(int use_waitpid)
>  
>  static void *test_pidfd_poll_leader_exit_thread(void *priv)
>  {
> -	ksft_print_msg("Child Thread: starting. pid %d tid %d ; and sleeping\n",
> +	ksft_print_msg("Child Thread: starting. pid %d tid %ld ; and sleeping\n",
>  			getpid(), syscall(SYS_gettid));
>  	sleep(CHILD_THREAD_MIN_WAIT);
> -	ksft_print_msg("Child Thread: DONE. pid %d tid %d\n", getpid(), syscall(SYS_gettid));
> +	ksft_print_msg("Child Thread: DONE. pid %d tid %ld\n", getpid(), syscall(SYS_gettid));
>  	return NULL;
>  }
>  
> @@ -492,7 +492,7 @@ static int child_poll_leader_exit_test(void *args)
>  {
>  	pthread_t t1, t2;
>  
> -	ksft_print_msg("Child: starting. pid %d tid %d\n", getpid(), syscall(SYS_gettid));
> +	ksft_print_msg("Child: starting. pid %d tid %ld\n", getpid(), syscall(SYS_gettid));
>  	pthread_create(&t1, NULL, test_pidfd_poll_leader_exit_thread, NULL);
>  	pthread_create(&t2, NULL, test_pidfd_poll_leader_exit_thread, NULL);
>  
> 

-- 
 i.

Re: [PATCH 4/6] selftests/pidfd: Fix ksft print formats

[Maciej Wieczór-Retman]

On 2023-08-28 at 14:01:18 +0300, Ilpo Järvinen wrote:
>On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:
>> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
>> ---
>>  tools/testing/selftests/pidfd/pidfd_fdinfo_test.c |  2 +-
>>  tools/testing/selftests/pidfd/pidfd_test.c        | 12 ++++++------
>>  2 files changed, 7 insertions(+), 7 deletions(-)
>> 
>> diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
>> index 4e86f927880c..01cc37bf611c 100644
>> --- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
>> +++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c
>> @@ -62,7 +62,7 @@ static void error_report(struct error *err, const char *test_name)
>>  		break;
>>  
>>  	case PIDFD_PASS:
>> -		ksft_test_result_pass("%s test: Passed\n");
>> +		ksft_test_result_pass("%s test: Passed\n", test_name);
>
>Missing test_name leads to crash so this looks a Fixes: class thing for
>me but you didn't provide one.

Thanks, I'll add this tag.

In my case it just gives a warning but compiles anyway:

pidfd_fdinfo_test.c: In function ‘error_report’:
pidfd_fdinfo_test.c:65:41: warning: format ‘%s’ expects a matching ‘char *’ argument [-Wformat=]
   65 |                 ksft_test_result_pass("%s test: Passed\n");
      |                                        ~^
      |                                         |
      |                                         char *

Could I be missing some strict config?

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH 2/6] selftests/cachestat: Fix print_cachestat format

[Nhat Pham]

On Mon, Aug 28, 2023 at 6:50 AM Wieczor-Retman, Maciej
<maciej.wieczor-retman@intel.com> wrote:
>
> The format specifier in printf() call expects long int variables and
> received long long int.
>
> Change format specifiers to long long int so they match passed
> variables.
>
> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
> ---
>  tools/testing/selftests/cachestat/test_cachestat.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
> index 77620e7ecf56..a7adf42afb20 100644
> --- a/tools/testing/selftests/cachestat/test_cachestat.c
> +++ b/tools/testing/selftests/cachestat/test_cachestat.c
> @@ -25,7 +25,7 @@ static const char * const dev_files[] = {
>  void print_cachestat(struct cachestat *cs)
>  {
>         ksft_print_msg(
> -       "Using cachestat: Cached: %lu, Dirty: %lu, Writeback: %lu, Evicted: %lu, Recently Evicted: %lu\n",
> +       "Using cachestat: Cached: %llu, Dirty: %llu, Writeback: %llu, Evicted: %llu, Recently Evicted: %llu\n",
The fields of struct cachestat should all be unsigned longs,
Is there a compiler warning that prompt this change?
>         cs->nr_cache, cs->nr_dirty, cs->nr_writeback,
>         cs->nr_evicted, cs->nr_recently_evicted);
>  }
> --
> 2.42.0
>

Re: [PATCH 2/6] selftests/cachestat: Fix print_cachestat format

[Nhat Pham]

On Mon, Aug 28, 2023 at 10:00 AM Nhat Pham <nphamcs@gmail.com> wrote:
>
> On Mon, Aug 28, 2023 at 6:50 AM Wieczor-Retman, Maciej
> <maciej.wieczor-retman@intel.com> wrote:
> >
> > The format specifier in printf() call expects long int variables and
> > received long long int.
> >
> > Change format specifiers to long long int so they match passed
> > variables.
> >
> > Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
> > ---
> >  tools/testing/selftests/cachestat/test_cachestat.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/cachestat/test_cachestat.c b/tools/testing/selftests/cachestat/test_cachestat.c
> > index 77620e7ecf56..a7adf42afb20 100644
> > --- a/tools/testing/selftests/cachestat/test_cachestat.c
> > +++ b/tools/testing/selftests/cachestat/test_cachestat.c
> > @@ -25,7 +25,7 @@ static const char * const dev_files[] = {
> >  void print_cachestat(struct cachestat *cs)
> >  {
> >         ksft_print_msg(
> > -       "Using cachestat: Cached: %lu, Dirty: %lu, Writeback: %lu, Evicted: %lu, Recently Evicted: %lu\n",
> > +       "Using cachestat: Cached: %llu, Dirty: %llu, Writeback: %llu, Evicted: %llu, Recently Evicted: %llu\n",
> The fields of struct cachestat should all be unsigned longs,
> Is there a compiler warning that prompt this change?
Ah nvm, it's _u64. %llu is probably the better format specifier indeed.

Acked-by: Nhat Pham <nphamcs@gmail.com>
> >         cs->nr_cache, cs->nr_dirty, cs->nr_writeback,
> >         cs->nr_evicted, cs->nr_recently_evicted);
> >  }
> > --
> > 2.42.0
> >

Re: [PATCH 5/6] selftests/sigaltstack: Fix wrong format specifier

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:

> The format specifier inside ksft printing function expects a long
> unsigned int but the passed variable is of unsigned int type.
> 
> Fix the format specifier so it matches the passed variable.
> 
> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
> ---
>  tools/testing/selftests/sigaltstack/sas.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/sigaltstack/sas.c b/tools/testing/selftests/sigaltstack/sas.c
> index 98d37cb744fb..07227fab1cc9 100644
> --- a/tools/testing/selftests/sigaltstack/sas.c
> +++ b/tools/testing/selftests/sigaltstack/sas.c
> @@ -111,7 +111,7 @@ int main(void)
>  
>  	/* Make sure more than the required minimum. */
>  	stack_size = getauxval(AT_MINSIGSTKSZ) + SIGSTKSZ;
> -	ksft_print_msg("[NOTE]\tthe stack size is %lu\n", stack_size);
> +	ksft_print_msg("[NOTE]\tthe stack size is %u\n", stack_size);
>  
>  	ksft_print_header();
>  	ksft_set_plan(3);

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH 6/6] selftests/kvm: Replace attribute with macro

[Ilpo Järvinen]

On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:

> The __printf() macro is used in many tools in the linux kernel to
> validate the format specifiers in functions that use printf. Some
> selftests use it without putting it in a macro definition and some tests
> import the kselftests.h header.

"Some" and yet this only converts one? Please be more precise in the 
wording.

> Use __printf() attribute instead of the full attribute since the macro
> is inside kselftests.h and the header is already imported.

IMO, this would be enough:

Use __printf() from kselftests.h instead of the full attribute.

Was there a reason why you didn't convert mm/pkey-helpers.h one?

-- 
 i.


> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
> ---
>  tools/testing/selftests/kvm/include/test_util.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index a6e9f215ce70..710a8a78e8ce 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -33,7 +33,7 @@ static inline int _no_printf(const char *format, ...) { return 0; }
>  #define pr_info(...) _no_printf(__VA_ARGS__)
>  #endif
>  
> -void print_skip(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
> +void __printf(1, 2) print_skip(const char *fmt, ...);
>  #define __TEST_REQUIRE(f, fmt, ...)				\
>  do {								\
>  	if (!(f))						\
> 

Re: [PATCH 3/6] selftests/openat2: Fix wrong format specifier

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]

On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:

> Ksft_print_msg() inside test_openat2_flags() uses the wrong format
> specifier for printing test.how->flags variable.
> 
> Change the format specifier to %llX so it matches the printed variable.
> 
> Signed-off-by: Wieczor-Retman, Maciej <maciej.wieczor-retman@intel.com>
> ---
>  tools/testing/selftests/openat2/openat2_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/openat2/openat2_test.c b/tools/testing/selftests/openat2/openat2_test.c
> index 7fb902099de4..9024754530b2 100644
> --- a/tools/testing/selftests/openat2/openat2_test.c
> +++ b/tools/testing/selftests/openat2/openat2_test.c
> @@ -300,7 +300,7 @@ void test_openat2_flags(void)
>  
>  			ksft_print_msg("openat2 unexpectedly returned ");
>  			if (fdpath)
> -				ksft_print_msg("%d['%s'] with %X (!= %X)\n",
> +				ksft_print_msg("%d['%s'] with %X (!= %llX)\n",
>  					       fd, fdpath, fdflags,
>  					       test->how.flags);
>  			else
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH 6/6] selftests/kvm: Replace attribute with macro

[Maciej Wieczór-Retman]

On 2023-08-30 at 15:22:57 +0300, Ilpo Järvinen wrote:
>On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:
>
>> The __printf() macro is used in many tools in the linux kernel to
>> validate the format specifiers in functions that use printf. Some
>> selftests use it without putting it in a macro definition and some tests
>> import the kselftests.h header.
>
>"Some" and yet this only converts one? Please be more precise in the 
>wording.

Okay, I'll mention them by subsystem.

>> Use __printf() attribute instead of the full attribute since the macro
>> is inside kselftests.h and the header is already imported.
>
>IMO, this would be enough:
>
>Use __printf() from kselftests.h instead of the full attribute.

Fair enough, I'll change the paragraph to that.

>Was there a reason why you didn't convert mm/pkey-helpers.h one?

Sorry, must have just missed it somehow. Thank you for pointing it out.

-- 
Kind regards
Maciej Wieczór-Retman

Re: [PATCH 6/6] selftests/kvm: Replace attribute with macro

[Andrew Jones]

On Wed, Aug 30, 2023 at 03:40:10PM +0200, Maciej Wieczór-Retman wrote:
> On 2023-08-30 at 15:22:57 +0300, Ilpo Järvinen wrote:
> >On Mon, 28 Aug 2023, Wieczor-Retman, Maciej wrote:
> >
> >> The __printf() macro is used in many tools in the linux kernel to
> >> validate the format specifiers in functions that use printf. Some
> >> selftests use it without putting it in a macro definition and some tests
> >> import the kselftests.h header.
> >
> >"Some" and yet this only converts one? Please be more precise in the 
> >wording.
> 
> Okay, I'll mention them by subsystem.
> 
> >> Use __printf() attribute instead of the full attribute since the macro
> >> is inside kselftests.h and the header is already imported.
> >
> >IMO, this would be enough:
> >
> >Use __printf() from kselftests.h instead of the full attribute.
> 
> Fair enough, I'll change the paragraph to that.

There are two in kvm selftests. test_assert(), a few lines down, also uses
the attribute.

Thanks,
drew

> 
> >Was there a reason why you didn't convert mm/pkey-helpers.h one?
> 
> Sorry, must have just missed it somehow. Thank you for pointing it out.
> 
> -- 
> Kind regards
> Maciej Wieczór-Retman