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

List overview All Threads
Download

newer

older

[PATCH bpf-next] selftests/bpf:...

[PATCH bpf-next v9] selftests/bpf:...

Wieczor-Retman, Maciej

28 Aug 2023 28 Aug '23

10:48 a.m.

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

Show replies by date

Wieczor-Retman, Maciej

28 Aug 28 Aug

10:49 a.m.

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

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

Wieczor-Retman, Maciej

10:49 a.m.

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

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

Nhat Pham

2 p.m.

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

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

Nhat Pham

2:04 p.m.

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

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

Wieczor-Retman, Maciej

10:49 a.m.

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

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

Ilpo Järvinen

30 Aug 30 Aug

12:25 p.m.

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

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.

Wieczor-Retman, Maciej

28 Aug 28 Aug

10:49 a.m.

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

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

Ilpo Järvinen

11:01 a.m.

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

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.

Maciej Wieczór-Retman

1:07 p.m.

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

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

Wieczor-Retman, Maciej

10:49 a.m.

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

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

Ilpo Järvinen

30 Aug 30 Aug

12:02 p.m.

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

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.

Wieczor-Retman, Maciej

28 Aug 28 Aug

10:49 a.m.

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

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

Ilpo Järvinen

30 Aug 30 Aug

12:22 p.m.

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

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)) \ >

Maciej Wieczór-Retman

1:40 p.m.

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

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

Andrew Jones

31 Aug 31 Aug

2:33 p.m.

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

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

263

days inactive

266

days old

linux-kselftest-mirror@lists.linaro.org

15 comments

participants

tags (0)

participants (5)

Andrew Jones
Ilpo Järvinen
Maciej Wieczór-Retman
Nhat Pham
Wieczor-Retman, Maciej