[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 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 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 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