lib/tests/printf_kunit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
From: Tamir Duberstein <tamird@gmail.com>
This produces better diagnostics when incorrect inputs are passed.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202512061600.89CKQ3ag-lkp@intel.com/
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
lib/tests/printf_kunit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/tests/printf_kunit.c b/lib/tests/printf_kunit.c
index 7617e5b8b02c..13e2e9afae3b 100644
--- a/lib/tests/printf_kunit.c
+++ b/lib/tests/printf_kunit.c
@@ -266,7 +266,7 @@ hash_pointer(struct kunit *kunittest)
KUNIT_EXPECT_MEMNEQ(kunittest, buf, PTR_STR, PTR_WIDTH);
}
-static void
+static void __printf(2, 3)
test_hashed(struct kunit *kunittest, const char *fmt, const void *p)
{
char buf[PLAIN_BUF_SIZE];
---
base-commit: 559e608c46553c107dbba19dae0854af7b219400
change-id: 20251206-printf-kunit-printf-attr-19369fc57bf0
Best regards,
--
Tamir Duberstein <tamird@gmail.com>
Hi Tamir,
kernel test robot noticed the following build errors:
[auto build test ERROR on 559e608c46553c107dbba19dae0854af7b219400]
url: https://github.com/intel-lab-lkp/linux/commits/Tamir-Duberstein/printf-add-__printf-attribute/20251206-212115
base: 559e608c46553c107dbba19dae0854af7b219400
patch link: https://lore.kernel.org/r/20251206-printf-kunit-printf-attr-v1-1-1682808b51d0%40gmail.com
patch subject: [PATCH] printf: add __printf attribute
config: um-allyesconfig (https://download.01.org/0day-ci/archive/20251207/202512070958.HMqvski6-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251207/202512070958.HMqvski6-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512070958.HMqvski6-lkp@intel.com/
All errors (new ones prefixed by >>):
>> lib/tests/printf_kunit.c:271:1: error: 'format' attribute argument 3 value '3' does not refer to a variable argument list
271 | {
| ^
vim +271 lib/tests/printf_kunit.c
707cc7280f452a lib/test_printf.c Rasmus Villemoes 2015-11-06 268
c23bf7282349cc lib/tests/printf_kunit.c Tamir Duberstein 2025-12-06 269 static void __printf(2, 3)
81a03aa9b88c5d lib/tests/printf_kunit.c Tamir Duberstein 2025-03-07 270 test_hashed(struct kunit *kunittest, const char *fmt, const void *p)
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 @271 {
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 272 char buf[PLAIN_BUF_SIZE];
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 273
81a03aa9b88c5d lib/tests/printf_kunit.c Tamir Duberstein 2025-03-07 274 plain_hash_to_buffer(kunittest, p, buf, PLAIN_BUF_SIZE);
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 275
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 276 test(buf, fmt, p);
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 277 }
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 278
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Tamir,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 559e608c46553c107dbba19dae0854af7b219400]
url: https://github.com/intel-lab-lkp/linux/commits/Tamir-Duberstein/printf-add-__printf-attribute/20251206-212115
base: 559e608c46553c107dbba19dae0854af7b219400
patch link: https://lore.kernel.org/r/20251206-printf-kunit-printf-attr-v1-1-1682808b51d0%40gmail.com
patch subject: [PATCH] printf: add __printf attribute
config: um-randconfig-001-20251207 (https://download.01.org/0day-ci/archive/20251207/202512070856.1syRb0ZL-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project a805147ac1ba123916de182babb0831fbb148756)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251207/202512070856.1syRb0ZL-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512070856.1syRb0ZL-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> lib/tests/printf_kunit.c:270:1: warning: GCC requires a function with the '__format__' attribute to be variadic [-Wgcc-compat]
270 | test_hashed(struct kunit *kunittest, const char *fmt, const void *p)
| ^
1 warning generated.
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for OF_GPIO
Depends on [n]: GPIOLIB [=y] && OF [=y] && HAS_IOMEM [=n]
Selected by [m]:
- REGULATOR_RT5133 [=m] && REGULATOR [=y] && I2C [=m] && GPIOLIB [=y] && OF [=y]
vim +/__format__ +270 lib/tests/printf_kunit.c
707cc7280f452a lib/test_printf.c Rasmus Villemoes 2015-11-06 268
c23bf7282349cc lib/tests/printf_kunit.c Tamir Duberstein 2025-12-06 269 static void __printf(2, 3)
81a03aa9b88c5d lib/tests/printf_kunit.c Tamir Duberstein 2025-03-07 @270 test_hashed(struct kunit *kunittest, const char *fmt, const void *p)
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 271 {
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 272 char buf[PLAIN_BUF_SIZE];
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 273
81a03aa9b88c5d lib/tests/printf_kunit.c Tamir Duberstein 2025-03-07 274 plain_hash_to_buffer(kunittest, p, buf, PLAIN_BUF_SIZE);
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 275
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 276 test(buf, fmt, p);
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 277 }
4d42c44727a062 lib/test_printf.c Andy Shevchenko 2018-12-04 278
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote:
> This produces better diagnostics when incorrect inputs are passed.
...
> -static void
> +static void __printf(2, 3)
3?!
I think it should be (2, 0). Yes, the both users call it with "%p..." in format
string, but the second parameter tells compiler to check the variadic
arguments, which are absent here. Changing 'const void *p' to '...' will align
it with the given __printf() attribute, but I don't know if this what we want.
> test_hashed(struct kunit *kunittest, const char *fmt, const void *p)
> {
> char buf[PLAIN_BUF_SIZE];
--
With Best Regards,
Andy Shevchenko
On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote: > > > This produces better diagnostics when incorrect inputs are passed. > > ... > > > -static void > > +static void __printf(2, 3) > > 3?! > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format > string, but the second parameter tells compiler to check the variadic > arguments, which are absent here. Changing 'const void *p' to '...' will align > it with the given __printf() attribute, but I don't know if this what we want. The second parameter is the first-to-check, it is not specific to variadic arguments. Using 0 means that no arguments are checkable, so the compiler only validates the format string itself and won’t diagnose mismatches with `p`. This works whether or not we later change `const void *p` to `...`. Cheers. Tamir
On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote: > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote: ... > > > -static void > > > +static void __printf(2, 3) > > > > 3?! > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format > > string, but the second parameter tells compiler to check the variadic > > arguments, which are absent here. Changing 'const void *p' to '...' will align > > it with the given __printf() attribute, but I don't know if this what we want. > > The second parameter is the first-to-check, it is not specific to > variadic arguments. Using 0 means that no arguments are checkable, so > the compiler only validates the format string itself and won’t > diagnose mismatches with `p`. This works whether or not we later > change `const void *p` to `...`. Yes, but this is fragile. As I explained it works only because we supply the format string stuck to "%p", anything else will require reconsidering the function prototypes. So, strictly speaking this should be (2, 0) if we leave const void *p as is. -- With Best Regards, Andy Shevchenko
On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote: > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote: > > ... > > > > > -static void > > > > +static void __printf(2, 3) > > > > > > 3?! > > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format > > > string, but the second parameter tells compiler to check the variadic > > > arguments, which are absent here. Changing 'const void *p' to '...' will align > > > it with the given __printf() attribute, but I don't know if this what we want. > > > > The second parameter is the first-to-check, it is not specific to > > variadic arguments. Using 0 means that no arguments are checkable, so > > the compiler only validates the format string itself and won’t > > diagnose mismatches with `p`. This works whether or not we later > > change `const void *p` to `...`. > > Yes, but this is fragile. As I explained it works only because we supply > the format string stuck to "%p", anything else will require reconsidering > the function prototypes. So, strictly speaking this should be (2, 0) if > we leave const void *p as is. > > -- > With Best Regards, > Andy Shevchenko I believe this is not correct. As I said, 0 means "do not check arguments" so only the format string will be checked. See the existing uses of this annotation in this file: static void __printf(7, 0) do_test(struct kunit *kunittest, const char *file, const int line, int bufsize, const char *expect, int elen, const char *fmt, va_list ap) and static void __printf(6, 7) __test(struct kunit *kunittest, const char *file, const int line, const char *expect, int elen, const char *fmt, ...) as you can see, 0 is used only when the arguments are not in the function prototype at all. When variadic arguments are present, N+1 is used. Cheers. Tamir
On Sat, Dec 06, 2025 at 12:52:53PM -0500, Tamir Duberstein wrote: > On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote: > > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote: ... > > > > > -static void > > > > > +static void __printf(2, 3) > > > > > > > > 3?! > > > > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format > > > > string, but the second parameter tells compiler to check the variadic > > > > arguments, which are absent here. Changing 'const void *p' to '...' will align > > > > it with the given __printf() attribute, but I don't know if this what we want. > > > > > > The second parameter is the first-to-check, it is not specific to > > > variadic arguments. Using 0 means that no arguments are checkable, so > > > the compiler only validates the format string itself and won’t > > > diagnose mismatches with `p`. This works whether or not we later > > > change `const void *p` to `...`. > > > > Yes, but this is fragile. As I explained it works only because we supply > > the format string stuck to "%p", anything else will require reconsidering > > the function prototypes. So, strictly speaking this should be (2, 0) if > > we leave const void *p as is. > > > I believe this is not correct. As I said, 0 means "do not check > arguments" so only the format string will be checked. See the existing > uses of this annotation in this file: > > static void __printf(7, 0) > do_test(struct kunit *kunittest, const char *file, const int line, int > bufsize, const char *expect, > int elen, const char *fmt, va_list ap) > > and > > static void __printf(6, 7) > __test(struct kunit *kunittest, const char *file, const int line, > const char *expect, int elen, > const char *fmt, ...) > > as you can see, 0 is used only when the arguments are not in the > function prototype at all. When variadic arguments are present, N+1 is > used. Yes to all what you said. And how does it object what I said? In the case you are trying to add __print(2, 3) the 3rd one is *not* a variadic argument. If you make it to be variadic, I will agree with __print(2, 3). Before that it doesn't look right to me even if it works. -- With Best Regards, Andy Shevchenko
On Sat, Dec 6, 2025 at 2:43 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Sat, Dec 06, 2025 at 12:52:53PM -0500, Tamir Duberstein wrote: > > On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote: > > > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote: > > ... > > > > > > > -static void > > > > > > +static void __printf(2, 3) > > > > > > > > > > 3?! > > > > > > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format > > > > > string, but the second parameter tells compiler to check the variadic > > > > > arguments, which are absent here. Changing 'const void *p' to '...' will align > > > > > it with the given __printf() attribute, but I don't know if this what we want. > > > > > > > > The second parameter is the first-to-check, it is not specific to > > > > variadic arguments. Using 0 means that no arguments are checkable, so > > > > the compiler only validates the format string itself and won’t > > > > diagnose mismatches with `p`. This works whether or not we later > > > > change `const void *p` to `...`. > > > > > > Yes, but this is fragile. As I explained it works only because we supply > > > the format string stuck to "%p", anything else will require reconsidering > > > the function prototypes. So, strictly speaking this should be (2, 0) if > > > we leave const void *p as is. > > > > > I believe this is not correct. As I said, 0 means "do not check > > arguments" so only the format string will be checked. See the existing > > uses of this annotation in this file: > > > > static void __printf(7, 0) > > do_test(struct kunit *kunittest, const char *file, const int line, int > > bufsize, const char *expect, > > int elen, const char *fmt, va_list ap) > > > > and > > > > static void __printf(6, 7) > > __test(struct kunit *kunittest, const char *file, const int line, > > const char *expect, int elen, > > const char *fmt, ...) > > > > as you can see, 0 is used only when the arguments are not in the > > function prototype at all. When variadic arguments are present, N+1 is > > used. > > Yes to all what you said. And how does it object what I said? In the case > you are trying to add __print(2, 3) the 3rd one is *not* a variadic argument. > If you make it to be variadic, I will agree with __print(2, 3). Before that > it doesn't look right to me even if it works. I addressed this in my first reply; the second parameter to `__print` is *not* specific to variadic functions. It can just as well be used for functions with a fixed number of arguments.
On Sat, Dec 06, 2025 at 02:57:48PM -0500, Tamir Duberstein wrote:
> On Sat, Dec 6, 2025 at 2:43 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Sat, Dec 06, 2025 at 12:52:53PM -0500, Tamir Duberstein wrote:
> > > On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote:
> > > > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote:
...
> > > > > > > -static void
> > > > > > > +static void __printf(2, 3)
> > > > > >
> > > > > > 3?!
> > > > > >
> > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format
> > > > > > string, but the second parameter tells compiler to check the variadic
> > > > > > arguments, which are absent here. Changing 'const void *p' to '...' will align
> > > > > > it with the given __printf() attribute, but I don't know if this what we want.
> > > > >
> > > > > The second parameter is the first-to-check, it is not specific to
> > > > > variadic arguments. Using 0 means that no arguments are checkable, so
> > > > > the compiler only validates the format string itself and won’t
> > > > > diagnose mismatches with `p`. This works whether or not we later
> > > > > change `const void *p` to `...`.
> > > >
> > > > Yes, but this is fragile. As I explained it works only because we supply
> > > > the format string stuck to "%p", anything else will require reconsidering
> > > > the function prototypes. So, strictly speaking this should be (2, 0) if
> > > > we leave const void *p as is.
> > > >
> > > I believe this is not correct. As I said, 0 means "do not check
> > > arguments" so only the format string will be checked. See the existing
> > > uses of this annotation in this file:
> > >
> > > static void __printf(7, 0)
> > > do_test(struct kunit *kunittest, const char *file, const int line, int
> > > bufsize, const char *expect,
> > > int elen, const char *fmt, va_list ap)
> > >
> > > and
> > >
> > > static void __printf(6, 7)
> > > __test(struct kunit *kunittest, const char *file, const int line,
> > > const char *expect, int elen,
> > > const char *fmt, ...)
> > >
> > > as you can see, 0 is used only when the arguments are not in the
> > > function prototype at all. When variadic arguments are present, N+1 is
> > > used.
> >
> > Yes to all what you said. And how does it object what I said? In the case
> > you are trying to add __print(2, 3) the 3rd one is *not* a variadic argument.
> > If you make it to be variadic, I will agree with __print(2, 3). Before that
> > it doesn't look right to me even if it works.
>
> I addressed this in my first reply; the second parameter to `__print`
> is *not* specific to variadic functions. It can just as well be used
> for functions with a fixed number of arguments.
$ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
ERROR:root:../lib/tests/printf_kunit.c:272:1: error: ‘format’ attribute argument 3 value ‘3’ does not refer to a variable argument list
272 | {
| ^
How did you compile it?
The GCC documentation
https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/Common-Function-Attributes.html#index-format-function-attribute
doesn't clearly say if the fixed-argument functions are eligible for the
__attribute__((format)). The parameter is called first-to-check, which
might imply that there is a second.
Additionally interesting discussion to read:
https://reviews.llvm.org/D112579
Seems it's feature of clang?
3147 As an extension to GCC's behavior, Clang accepts the ``format`` attribute on
3148 non-variadic functions. Clang checks non-variadic format functions for the same
3149 classes of issues that can be found on variadic functions, as controlled by the
3150 same warning flags, except that the types of formatted arguments is forced by
3151 the function signature. For example:
Seems to me for now it has to be __printf(2, 0) or you need to put some special
pragma:s or so around the function to make it work for clang differently.
--
With Best Regards,
Andy Shevchenko
On Sat, Dec 6, 2025 at 4:45 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Sat, Dec 06, 2025 at 02:57:48PM -0500, Tamir Duberstein wrote:
> > On Sat, Dec 6, 2025 at 2:43 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Sat, Dec 06, 2025 at 12:52:53PM -0500, Tamir Duberstein wrote:
> > > > On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote:
> > > > > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko
> > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote:
>
> ...
>
> > > > > > > > -static void
> > > > > > > > +static void __printf(2, 3)
> > > > > > >
> > > > > > > 3?!
> > > > > > >
> > > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format
> > > > > > > string, but the second parameter tells compiler to check the variadic
> > > > > > > arguments, which are absent here. Changing 'const void *p' to '...' will align
> > > > > > > it with the given __printf() attribute, but I don't know if this what we want.
> > > > > >
> > > > > > The second parameter is the first-to-check, it is not specific to
> > > > > > variadic arguments. Using 0 means that no arguments are checkable, so
> > > > > > the compiler only validates the format string itself and won’t
> > > > > > diagnose mismatches with `p`. This works whether or not we later
> > > > > > change `const void *p` to `...`.
> > > > >
> > > > > Yes, but this is fragile. As I explained it works only because we supply
> > > > > the format string stuck to "%p", anything else will require reconsidering
> > > > > the function prototypes. So, strictly speaking this should be (2, 0) if
> > > > > we leave const void *p as is.
> > > > >
> > > > I believe this is not correct. As I said, 0 means "do not check
> > > > arguments" so only the format string will be checked. See the existing
> > > > uses of this annotation in this file:
> > > >
> > > > static void __printf(7, 0)
> > > > do_test(struct kunit *kunittest, const char *file, const int line, int
> > > > bufsize, const char *expect,
> > > > int elen, const char *fmt, va_list ap)
> > > >
> > > > and
> > > >
> > > > static void __printf(6, 7)
> > > > __test(struct kunit *kunittest, const char *file, const int line,
> > > > const char *expect, int elen,
> > > > const char *fmt, ...)
> > > >
> > > > as you can see, 0 is used only when the arguments are not in the
> > > > function prototype at all. When variadic arguments are present, N+1 is
> > > > used.
> > >
> > > Yes to all what you said. And how does it object what I said? In the case
> > > you are trying to add __print(2, 3) the 3rd one is *not* a variadic argument.
> > > If you make it to be variadic, I will agree with __print(2, 3). Before that
> > > it doesn't look right to me even if it works.
> >
> > I addressed this in my first reply; the second parameter to `__print`
> > is *not* specific to variadic functions. It can just as well be used
> > for functions with a fixed number of arguments.
>
> $ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
> ERROR:root:../lib/tests/printf_kunit.c:272:1: error: ‘format’ attribute argument 3 value ‘3’ does not refer to a variable argument list
> 272 | {
> | ^
>
> How did you compile it?
>
> The GCC documentation
> https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/Common-Function-Attributes.html#index-format-function-attribute
> doesn't clearly say if the fixed-argument functions are eligible for the
> __attribute__((format)). The parameter is called first-to-check, which
> might imply that there is a second.
>
> Additionally interesting discussion to read:
> https://reviews.llvm.org/D112579
>
> Seems it's feature of clang?
>
> 3147 As an extension to GCC's behavior, Clang accepts the ``format`` attribute on
> 3148 non-variadic functions. Clang checks non-variadic format functions for the same
> 3149 classes of issues that can be found on variadic functions, as controlled by the
> 3150 same warning flags, except that the types of formatted arguments is forced by
> 3151 the function signature. For example:
>
> Seems to me for now it has to be __printf(2, 0) or you need to put some special
> pragma:s or so around the function to make it work for clang differently.
Ah, thanks for digging that up - and as confirmed by LKP you are right
of course.
Since it doesn't make much sense to make this function variadic, I
think the best we can do is a macro wrapper that combines this
function with `no_printk`. Something like
#define test_hashed(kunittest, fmt, p) \
do { \
if (0) \
no_printk(fmt, p); \
__test_hashed(kunittest, fmt, p);\
} while (0)
That would give us better diagnostics, but is more complex (and more
lines of code than just repeating this function's body twice, which
would also give good diagnostics). I think the best thing to do is just
to ignore the report that prompted me to look into this. Please let me
know if you disagree.
My best,
Tamir
On Sun, Dec 07, 2025 at 08:32:53PM -0500, Tamir Duberstein wrote:
> On Sat, Dec 6, 2025 at 4:45 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Sat, Dec 06, 2025 at 02:57:48PM -0500, Tamir Duberstein wrote:
> > > On Sat, Dec 6, 2025 at 2:43 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Sat, Dec 06, 2025 at 12:52:53PM -0500, Tamir Duberstein wrote:
> > > > > On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote:
> > > > > > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko
> > > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote:
...
> > > > > > > > > -static void
> > > > > > > > > +static void __printf(2, 3)
> > > > > > > >
> > > > > > > > 3?!
> > > > > > > >
> > > > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format
> > > > > > > > string, but the second parameter tells compiler to check the variadic
> > > > > > > > arguments, which are absent here. Changing 'const void *p' to '...' will align
> > > > > > > > it with the given __printf() attribute, but I don't know if this what we want.
> > > > > > >
> > > > > > > The second parameter is the first-to-check, it is not specific to
> > > > > > > variadic arguments. Using 0 means that no arguments are checkable, so
> > > > > > > the compiler only validates the format string itself and won’t
> > > > > > > diagnose mismatches with `p`. This works whether or not we later
> > > > > > > change `const void *p` to `...`.
> > > > > >
> > > > > > Yes, but this is fragile. As I explained it works only because we supply
> > > > > > the format string stuck to "%p", anything else will require reconsidering
> > > > > > the function prototypes. So, strictly speaking this should be (2, 0) if
> > > > > > we leave const void *p as is.
> > > > > >
> > > > > I believe this is not correct. As I said, 0 means "do not check
> > > > > arguments" so only the format string will be checked. See the existing
> > > > > uses of this annotation in this file:
> > > > >
> > > > > static void __printf(7, 0)
> > > > > do_test(struct kunit *kunittest, const char *file, const int line, int
> > > > > bufsize, const char *expect,
> > > > > int elen, const char *fmt, va_list ap)
> > > > >
> > > > > and
> > > > >
> > > > > static void __printf(6, 7)
> > > > > __test(struct kunit *kunittest, const char *file, const int line,
> > > > > const char *expect, int elen,
> > > > > const char *fmt, ...)
> > > > >
> > > > > as you can see, 0 is used only when the arguments are not in the
> > > > > function prototype at all. When variadic arguments are present, N+1 is
> > > > > used.
> > > >
> > > > Yes to all what you said. And how does it object what I said? In the case
> > > > you are trying to add __print(2, 3) the 3rd one is *not* a variadic argument.
> > > > If you make it to be variadic, I will agree with __print(2, 3). Before that
> > > > it doesn't look right to me even if it works.
> > >
> > > I addressed this in my first reply; the second parameter to `__print`
> > > is *not* specific to variadic functions. It can just as well be used
> > > for functions with a fixed number of arguments.
> >
> > $ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
> > ERROR:root:../lib/tests/printf_kunit.c:272:1: error: ‘format’ attribute argument 3 value ‘3’ does not refer to a variable argument list
> > 272 | {
> > | ^
> >
> > How did you compile it?
> >
> > The GCC documentation
> > https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/Common-Function-Attributes.html#index-format-function-attribute
> > doesn't clearly say if the fixed-argument functions are eligible for the
> > __attribute__((format)). The parameter is called first-to-check, which
> > might imply that there is a second.
> >
> > Additionally interesting discussion to read:
> > https://reviews.llvm.org/D112579
> >
> > Seems it's feature of clang?
> >
> > 3147 As an extension to GCC's behavior, Clang accepts the ``format`` attribute on
> > 3148 non-variadic functions. Clang checks non-variadic format functions for the same
> > 3149 classes of issues that can be found on variadic functions, as controlled by the
> > 3150 same warning flags, except that the types of formatted arguments is forced by
> > 3151 the function signature. For example:
> >
> > Seems to me for now it has to be __printf(2, 0) or you need to put some special
> > pragma:s or so around the function to make it work for clang differently.
>
> Ah, thanks for digging that up - and as confirmed by LKP you are right
> of course.
>
> Since it doesn't make much sense to make this function variadic, I
> think the best we can do is a macro wrapper that combines this
> function with `no_printk`. Something like
>
> #define test_hashed(kunittest, fmt, p) \
> do { \
> if (0) \
> no_printk(fmt, p); \
> __test_hashed(kunittest, fmt, p);\
> } while (0)
I am not sure about a macro approach.
> That would give us better diagnostics, but is more complex (and more
> lines of code than just repeating this function's body twice, which
> would also give good diagnostics). I think the best thing to do is just
> to ignore the report that prompted me to look into this. Please let me
> know if you disagree.
I think we may not ignore the report as it breaks builds in some cases.
As I said
- __printf(2, 0) for now
- and perhaps a comment on top to explain the clang approach that may cope
with fixed-argument functions for -Wformat (you can even put a link to that
LLVM discussion about the feature).
--
With Best Regards,
Andy Shevchenko
On Mon 2025-12-08 15:30:53, Andy Shevchenko wrote:
> On Sun, Dec 07, 2025 at 08:32:53PM -0500, Tamir Duberstein wrote:
> > On Sat, Dec 6, 2025 at 4:45 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Sat, Dec 06, 2025 at 02:57:48PM -0500, Tamir Duberstein wrote:
> > > > On Sat, Dec 6, 2025 at 2:43 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Sat, Dec 06, 2025 at 12:52:53PM -0500, Tamir Duberstein wrote:
> > > > > > On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko
> > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote:
> > > > > > > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko
> > > > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote:
>
> ...
>
> > > > > > > > > > -static void
> > > > > > > > > > +static void __printf(2, 3)
> > > > > > > > >
> > > > > > > > > 3?!
> > > > > > > > >
> > > > > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format
> > > > > > > > > string, but the second parameter tells compiler to check the variadic
> > > > > > > > > arguments, which are absent here. Changing 'const void *p' to '...' will align
> > > > > > > > > it with the given __printf() attribute, but I don't know if this what we want.
> > > > > > > >
> > > > > > > > The second parameter is the first-to-check, it is not specific to
> > > > > > > > variadic arguments. Using 0 means that no arguments are checkable, so
> > > > > > > > the compiler only validates the format string itself and won’t
> > > > > > > > diagnose mismatches with `p`. This works whether or not we later
> > > > > > > > change `const void *p` to `...`.
> > > > > > >
> > > > > > > Yes, but this is fragile. As I explained it works only because we supply
> > > > > > > the format string stuck to "%p", anything else will require reconsidering
> > > > > > > the function prototypes. So, strictly speaking this should be (2, 0) if
> > > > > > > we leave const void *p as is.
> > > > > > >
> > > > > > I believe this is not correct. As I said, 0 means "do not check
> > > > > > arguments" so only the format string will be checked. See the existing
> > > > > > uses of this annotation in this file:
> > > > > >
> > > > > > static void __printf(7, 0)
> > > > > > do_test(struct kunit *kunittest, const char *file, const int line, int
> > > > > > bufsize, const char *expect,
> > > > > > int elen, const char *fmt, va_list ap)
> > > > > >
> > > > > > and
> > > > > >
> > > > > > static void __printf(6, 7)
> > > > > > __test(struct kunit *kunittest, const char *file, const int line,
> > > > > > const char *expect, int elen,
> > > > > > const char *fmt, ...)
> > > > > >
> > > > > > as you can see, 0 is used only when the arguments are not in the
> > > > > > function prototype at all. When variadic arguments are present, N+1 is
> > > > > > used.
> > > > >
> > > > > Yes to all what you said. And how does it object what I said? In the case
> > > > > you are trying to add __print(2, 3) the 3rd one is *not* a variadic argument.
> > > > > If you make it to be variadic, I will agree with __print(2, 3). Before that
> > > > > it doesn't look right to me even if it works.
> > > >
> > > > I addressed this in my first reply; the second parameter to `__print`
> > > > is *not* specific to variadic functions. It can just as well be used
> > > > for functions with a fixed number of arguments.
> > >
> > > $ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
> > > ERROR:root:../lib/tests/printf_kunit.c:272:1: error: ‘format’ attribute argument 3 value ‘3’ does not refer to a variable argument list
> > > 272 | {
> > > | ^
> > >
> > > How did you compile it?
> > >
> > > The GCC documentation
> > > https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/Common-Function-Attributes.html#index-format-function-attribute
> > > doesn't clearly say if the fixed-argument functions are eligible for the
> > > __attribute__((format)). The parameter is called first-to-check, which
> > > might imply that there is a second.
> > >
> > > Additionally interesting discussion to read:
> > > https://reviews.llvm.org/D112579
> > >
> > > Seems it's feature of clang?
> > >
> > > 3147 As an extension to GCC's behavior, Clang accepts the ``format`` attribute on
> > > 3148 non-variadic functions. Clang checks non-variadic format functions for the same
> > > 3149 classes of issues that can be found on variadic functions, as controlled by the
> > > 3150 same warning flags, except that the types of formatted arguments is forced by
> > > 3151 the function signature. For example:
> > >
> > > Seems to me for now it has to be __printf(2, 0) or you need to put some special
> > > pragma:s or so around the function to make it work for clang differently.
> >
> > Ah, thanks for digging that up - and as confirmed by LKP you are right
> > of course.
> >
> > Since it doesn't make much sense to make this function variadic, I
> > think the best we can do is a macro wrapper that combines this
> > function with `no_printk`. Something like
> >
> > #define test_hashed(kunittest, fmt, p) \
> > do { \
> > if (0) \
> > no_printk(fmt, p); \
> > __test_hashed(kunittest, fmt, p);\
> > } while (0)
>
IMHO, this is is not worth it. test_hashed(kunittest, fmt, p) calls
test(buf, fmt, p). It goes down to __test() which does the format
check:
static void __printf(6, 7)
__test(struct kunit *kunittest, const char *file, const int line, const char *expect, int elen,
const char *fmt, ...)
> > That would give us better diagnostics, but is more complex (and more
> > lines of code than just repeating this function's body twice, which
> > would also give good diagnostics). I think the best thing to do is just
> > to ignore the report that prompted me to look into this. Please let me
> > know if you disagree.
>
> I think we may not ignore the report as it breaks builds in some cases.
> As I said
>
> - __printf(2, 0) for now
>
> - and perhaps a comment on top to explain the clang approach that may cope
> with fixed-argument functions for -Wformat (you can even put a link to that
> LLVM discussion about the feature).
I personally prefer this way. We just need to calm down the warning.
The proper check is done by the nested test()...
Best Regards,
Petr
On Mon, Dec 8, 2025 at 9:06 AM Petr Mladek <pmladek@suse.com> wrote:
>
> On Mon 2025-12-08 15:30:53, Andy Shevchenko wrote:
> > On Sun, Dec 07, 2025 at 08:32:53PM -0500, Tamir Duberstein wrote:
> > > On Sat, Dec 6, 2025 at 4:45 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Sat, Dec 06, 2025 at 02:57:48PM -0500, Tamir Duberstein wrote:
> > > > > On Sat, Dec 6, 2025 at 2:43 PM Andy Shevchenko
> > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > On Sat, Dec 06, 2025 at 12:52:53PM -0500, Tamir Duberstein wrote:
> > > > > > > On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko
> > > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote:
> > > > > > > > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko
> > > > > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote:
> >
> > ...
> >
> > > > > > > > > > > -static void
> > > > > > > > > > > +static void __printf(2, 3)
> > > > > > > > > >
> > > > > > > > > > 3?!
> > > > > > > > > >
> > > > > > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format
> > > > > > > > > > string, but the second parameter tells compiler to check the variadic
> > > > > > > > > > arguments, which are absent here. Changing 'const void *p' to '...' will align
> > > > > > > > > > it with the given __printf() attribute, but I don't know if this what we want.
> > > > > > > > >
> > > > > > > > > The second parameter is the first-to-check, it is not specific to
> > > > > > > > > variadic arguments. Using 0 means that no arguments are checkable, so
> > > > > > > > > the compiler only validates the format string itself and won’t
> > > > > > > > > diagnose mismatches with `p`. This works whether or not we later
> > > > > > > > > change `const void *p` to `...`.
> > > > > > > >
> > > > > > > > Yes, but this is fragile. As I explained it works only because we supply
> > > > > > > > the format string stuck to "%p", anything else will require reconsidering
> > > > > > > > the function prototypes. So, strictly speaking this should be (2, 0) if
> > > > > > > > we leave const void *p as is.
> > > > > > > >
> > > > > > > I believe this is not correct. As I said, 0 means "do not check
> > > > > > > arguments" so only the format string will be checked. See the existing
> > > > > > > uses of this annotation in this file:
> > > > > > >
> > > > > > > static void __printf(7, 0)
> > > > > > > do_test(struct kunit *kunittest, const char *file, const int line, int
> > > > > > > bufsize, const char *expect,
> > > > > > > int elen, const char *fmt, va_list ap)
> > > > > > >
> > > > > > > and
> > > > > > >
> > > > > > > static void __printf(6, 7)
> > > > > > > __test(struct kunit *kunittest, const char *file, const int line,
> > > > > > > const char *expect, int elen,
> > > > > > > const char *fmt, ...)
> > > > > > >
> > > > > > > as you can see, 0 is used only when the arguments are not in the
> > > > > > > function prototype at all. When variadic arguments are present, N+1 is
> > > > > > > used.
> > > > > >
> > > > > > Yes to all what you said. And how does it object what I said? In the case
> > > > > > you are trying to add __print(2, 3) the 3rd one is *not* a variadic argument.
> > > > > > If you make it to be variadic, I will agree with __print(2, 3). Before that
> > > > > > it doesn't look right to me even if it works.
> > > > >
> > > > > I addressed this in my first reply; the second parameter to `__print`
> > > > > is *not* specific to variadic functions. It can just as well be used
> > > > > for functions with a fixed number of arguments.
> > > >
> > > > $ make all compile_commands.json scripts_gdb ARCH=um O=.kunit --jobs=48
> > > > ERROR:root:../lib/tests/printf_kunit.c:272:1: error: ‘format’ attribute argument 3 value ‘3’ does not refer to a variable argument list
> > > > 272 | {
> > > > | ^
> > > >
> > > > How did you compile it?
> > > >
> > > > The GCC documentation
> > > > https://gcc.gnu.org/onlinedocs/gcc-15.2.0/gcc/Common-Function-Attributes.html#index-format-function-attribute
> > > > doesn't clearly say if the fixed-argument functions are eligible for the
> > > > __attribute__((format)). The parameter is called first-to-check, which
> > > > might imply that there is a second.
> > > >
> > > > Additionally interesting discussion to read:
> > > > https://reviews.llvm.org/D112579
> > > >
> > > > Seems it's feature of clang?
> > > >
> > > > 3147 As an extension to GCC's behavior, Clang accepts the ``format`` attribute on
> > > > 3148 non-variadic functions. Clang checks non-variadic format functions for the same
> > > > 3149 classes of issues that can be found on variadic functions, as controlled by the
> > > > 3150 same warning flags, except that the types of formatted arguments is forced by
> > > > 3151 the function signature. For example:
> > > >
> > > > Seems to me for now it has to be __printf(2, 0) or you need to put some special
> > > > pragma:s or so around the function to make it work for clang differently.
> > >
> > > Ah, thanks for digging that up - and as confirmed by LKP you are right
> > > of course.
> > >
> > > Since it doesn't make much sense to make this function variadic, I
> > > think the best we can do is a macro wrapper that combines this
> > > function with `no_printk`. Something like
> > >
> > > #define test_hashed(kunittest, fmt, p) \
> > > do { \
> > > if (0) \
> > > no_printk(fmt, p); \
> > > __test_hashed(kunittest, fmt, p);\
> > > } while (0)
> >
>
> IMHO, this is is not worth it. test_hashed(kunittest, fmt, p) calls
> test(buf, fmt, p). It goes down to __test() which does the format
> check:
>
> static void __printf(6, 7)
> __test(struct kunit *kunittest, const char *file, const int line, const char *expect, int elen,
> const char *fmt, ...)
>
>
> > > That would give us better diagnostics, but is more complex (and more
> > > lines of code than just repeating this function's body twice, which
> > > would also give good diagnostics). I think the best thing to do is just
> > > to ignore the report that prompted me to look into this. Please let me
> > > know if you disagree.
> >
> > I think we may not ignore the report as it breaks builds in some cases.
> > As I said
> >
> > - __printf(2, 0) for now
> >
> > - and perhaps a comment on top to explain the clang approach that may cope
> > with fixed-argument functions for -Wformat (you can even put a link to that
> > LLVM discussion about the feature).
>
> I personally prefer this way. We just need to calm down the warning.
> The proper check is done by the nested test()...
The nested `__test()` call cannot do the proper check because it cannot
see the format string. Right?
On Mon 2025-12-08 16:07:28, Tamir Duberstein wrote:
> On Mon, Dec 8, 2025 at 9:06 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> > On Mon 2025-12-08 15:30:53, Andy Shevchenko wrote:
> > > On Sun, Dec 07, 2025 at 08:32:53PM -0500, Tamir Duberstein wrote:
> > > > On Sat, Dec 6, 2025 at 4:45 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Sat, Dec 06, 2025 at 02:57:48PM -0500, Tamir Duberstein wrote:
> > > > > > On Sat, Dec 6, 2025 at 2:43 PM Andy Shevchenko
> > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > On Sat, Dec 06, 2025 at 12:52:53PM -0500, Tamir Duberstein wrote:
> > > > > > > > On Sat, Dec 6, 2025 at 12:49 PM Andy Shevchenko
> > > > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > > > On Sat, Dec 06, 2025 at 12:13:34PM -0500, Tamir Duberstein wrote:
> > > > > > > > > > On Sat, Dec 6, 2025 at 11:11 AM Andy Shevchenko
> > > > > > > > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > > > > > > > On Sat, Dec 06, 2025 at 08:19:09AM -0500, Tamir Duberstein wrote:
> > >
> > > ...
> > >
> > > > > > > > > > > > -static void
> > > > > > > > > > > > +static void __printf(2, 3)
> > > > > > > > > > >
> > > > > > > > > > > 3?!
> > > > > > > > > > >
> > > > > > > > > > > I think it should be (2, 0). Yes, the both users call it with "%p..." in format
> > > > > > > > > > > string, but the second parameter tells compiler to check the variadic
> > > > > > > > > > > arguments, which are absent here. Changing 'const void *p' to '...' will align
> > > > > > > > > > > it with the given __printf() attribute, but I don't know if this what we want.
> > > > > > > > > >
> > > > > > > > > > The second parameter is the first-to-check, it is not specific to
> > > > > > > > > > variadic arguments. Using 0 means that no arguments are checkable, so
> > > > > > > > > > the compiler only validates the format string itself and won’t
> > > > > > > > > > diagnose mismatches with `p`. This works whether or not we later
> > > > > > > > > > change `const void *p` to `...`.
> > > > > > > > >
> > > > > > > > > Yes, but this is fragile. As I explained it works only because we supply
> > > > > > > > > the format string stuck to "%p", anything else will require reconsidering
> > > > > > > > > the function prototypes. So, strictly speaking this should be (2, 0) if
> > > > > > > > > we leave const void *p as is.
> > > > > > > > >
> > > > > > > > I believe this is not correct. As I said, 0 means "do not check
> > > > > > > > arguments" so only the format string will be checked. See the existing
> > > > > > > > uses of this annotation in this file:
> > > > > > > >
> > > > > > > > static void __printf(7, 0)
> > > > > > > > do_test(struct kunit *kunittest, const char *file, const int line, int
> > > > > > > > bufsize, const char *expect,
> > > > > > > > int elen, const char *fmt, va_list ap)
> > > > > > > >
> > > > > > > > and
> > > > > > > >
> > > > > > > > static void __printf(6, 7)
> > > > > > > > __test(struct kunit *kunittest, const char *file, const int line,
> > > > > > > > const char *expect, int elen,
> > > > > > > > const char *fmt, ...)
> > > > >
> > > > Since it doesn't make much sense to make this function variadic, I
> > > > think the best we can do is a macro wrapper that combines this
> > > > function with `no_printk`. Something like
> > > >
> > > > #define test_hashed(kunittest, fmt, p) \
> > > > do { \
> > > > if (0) \
> > > > no_printk(fmt, p); \
> > > > __test_hashed(kunittest, fmt, p);\
> > > > } while (0)
> > >
> >
> > IMHO, this is is not worth it. test_hashed(kunittest, fmt, p) calls
> > test(buf, fmt, p). It goes down to __test() which does the format
> > check:
> >
> > static void __printf(6, 7)
> > __test(struct kunit *kunittest, const char *file, const int line, const char *expect, int elen,
> > const char *fmt, ...)
> >
> >
> > > > That would give us better diagnostics, but is more complex (and more
> > > > lines of code than just repeating this function's body twice, which
> > > > would also give good diagnostics). I think the best thing to do is just
> > > > to ignore the report that prompted me to look into this. Please let me
> > > > know if you disagree.
> > >
> > > I think we may not ignore the report as it breaks builds in some cases.
> > > As I said
> > >
> > > - __printf(2, 0) for now
> > >
> > > - and perhaps a comment on top to explain the clang approach that may cope
> > > with fixed-argument functions for -Wformat (you can even put a link to that
> > > LLVM discussion about the feature).
> >
> > I personally prefer this way. We just need to calm down the warning.
> > The proper check is done by the nested test()...
>
> The nested `__test()` call cannot do the proper check because it cannot
> see the format string. Right?
IMHO, it does see the format string. It is defined the following way:
static void __printf(6, 7)
__test(struct kunit *kunittest, const char *file, const int line, const char *expect, int elen,
const char *fmt, ...)
#define test(expect, fmt, ...) \
__test(kunittest, __FILE__, __LINE__, expect, strlen(expect), fmt, ##__VA_ARGS__)
static void
test_hashed(struct kunit *kunittest, const char *fmt, const void *p)
{
char buf[PLAIN_BUF_SIZE];
plain_hash_to_buffer(kunittest, p, buf, PLAIN_BUF_SIZE);
test(buf, fmt, p);
}
Now, let's get for example:
test_hashed(kunittest, "%p", PTR_INVALID);
it calls:
test(buf, "%p", PTR_INVALID);
which is exapnded to:
__test(kunittest, file, line, buf, strlen(buf), "%p", PTR_INVALID)
so, it gets the same printf arguments as the original test_hashed,
namely:
%p, PTR_INVALID
Or do I miss anything, please?
You might argue that it works by chance and that it might change in the
future. But I have hard times to imagine it. test_hashed() is just
a wrapper around "test()". The only purpose is to fill "buf" with
the expected outcome.
If any refactoring is needed in the future. The __printf() macros
would need some refactoring as well.
Best Regards,
Petr
© 2016 - 2025 Red Hat, Inc.