linux-trace-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required
@ 2025-03-21 14:40 Andy Shevchenko
  2025-03-21 14:40 ` [PATCH v2 1/6] seq_buf: Mark binary printing functions with __printf() attribute Andy Shevchenko
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Andy Shevchenko @ 2025-03-21 14:40 UTC (permalink / raw)
  To: Andy Shevchenko, Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel
  Cc: Andy Shevchenko, Mathieu Desnoyers, Andrew Morton, Petr Mladek,
	Rasmus Villemoes, Sergey Senozhatsky

This whole series started from a simple fix (see the last patch)
to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
(note, that CONFIG_WERROR=y and all warnings break the build!)
down to a rabbit hole.

However starting from v2 the last patch doesn't require the first
part, I prefer still to have them since the functions, while being
_binary_ printf()-like, are still printf()-like. It also puts in align
the tracing stuff with the rest and fixes the wrong parameter value.

These first 4 patches are organised in a strict order and can't be
reshuffled, otherwise it will produce a warnings in between.

I believe the best route for the series is printk tree with immutable
tag or branch for the others.

Alternatively the first 4 patches can be applied first as they
are pretty much straightforward. They also can be squashed to one
(as the same topic behind), but it all is up to the respective
maintainers.

In v2:
- split out patch 5 (Rasmus)
- rewritten the approach for the va_format() fix (Rasmus)
- amended tracing patch (removed a blank line and a __printf() in C file)

Andy Shevchenko (6):
  seq_buf: Mark binary printing functions with __printf() attribute
  seq_file: Mark binary printing functions with __printf() attribute
  tracing: Mark binary printing functions with __printf() attribute
  vsnprintf: Mark binary printing functions with __printf() attribute
  vsnprintf: Drop unused const char fmt * in va_format()
  vsnprintf: Silence false positive GCC warning for va_format()

 include/linux/seq_buf.h   |  4 ++--
 include/linux/seq_file.h  |  1 +
 include/linux/string.h    |  4 ++--
 include/linux/trace.h     |  4 ++--
 include/linux/trace_seq.h |  8 ++++----
 kernel/trace/trace.c      | 11 +++--------
 kernel/trace/trace.h      | 16 +++++++++-------
 lib/vsprintf.c            |  9 +++++++--
 8 files changed, 30 insertions(+), 27 deletions(-)

-- 
2.47.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/6] seq_buf: Mark binary printing functions with __printf() attribute
  2025-03-21 14:40 [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Andy Shevchenko
@ 2025-03-21 14:40 ` Andy Shevchenko
  2025-03-21 14:40 ` [PATCH v2 2/6] seq_file: " Andy Shevchenko
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2025-03-21 14:40 UTC (permalink / raw)
  To: Andy Shevchenko, Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel
  Cc: Andy Shevchenko, Mathieu Desnoyers, Andrew Morton, Petr Mladek,
	Rasmus Villemoes, Sergey Senozhatsky

Binary printing functions are using printf() type of format, and compiler
is not happy about them as is:

lib/seq_buf.c:162:17: error: function ‘seq_buf_bprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]

Fix the compilation errors by adding __printf() attribute.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/seq_buf.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index fe41da005970..52791e070506 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -167,8 +167,8 @@ extern int seq_buf_hex_dump(struct seq_buf *s, const char *prefix_str,
 			    const void *buf, size_t len, bool ascii);
 
 #ifdef CONFIG_BINARY_PRINTF
-extern int
-seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary);
+__printf(2, 0)
+int seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary);
 #endif
 
 void seq_buf_do_printk(struct seq_buf *s, const char *lvl);
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 2/6] seq_file: Mark binary printing functions with __printf() attribute
  2025-03-21 14:40 [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Andy Shevchenko
  2025-03-21 14:40 ` [PATCH v2 1/6] seq_buf: Mark binary printing functions with __printf() attribute Andy Shevchenko
@ 2025-03-21 14:40 ` Andy Shevchenko
  2025-03-21 14:40 ` [PATCH v2 3/6] tracing: " Andy Shevchenko
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2025-03-21 14:40 UTC (permalink / raw)
  To: Andy Shevchenko, Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel
  Cc: Andy Shevchenko, Mathieu Desnoyers, Andrew Morton, Petr Mladek,
	Rasmus Villemoes, Sergey Senozhatsky

Binary printing functions are using printf() type of format, and compiler
is not happy about them as is:

fs/seq_file.c:418:35: error: function ‘seq_bprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]

Fix the compilation errors by adding __printf() attribute.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/seq_file.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/linux/seq_file.h b/include/linux/seq_file.h
index 2fb266ea69fa..d6ebf0596510 100644
--- a/include/linux/seq_file.h
+++ b/include/linux/seq_file.h
@@ -181,6 +181,7 @@ int seq_open_private(struct file *, const struct seq_operations *, int);
 int seq_release_private(struct inode *, struct file *);
 
 #ifdef CONFIG_BINARY_PRINTF
+__printf(2, 0)
 void seq_bprintf(struct seq_file *m, const char *f, const u32 *binary);
 #endif
 
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 3/6] tracing: Mark binary printing functions with __printf() attribute
  2025-03-21 14:40 [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Andy Shevchenko
  2025-03-21 14:40 ` [PATCH v2 1/6] seq_buf: Mark binary printing functions with __printf() attribute Andy Shevchenko
  2025-03-21 14:40 ` [PATCH v2 2/6] seq_file: " Andy Shevchenko
@ 2025-03-21 14:40 ` Andy Shevchenko
  2025-03-21 14:40 ` [PATCH v2 4/6] vsnprintf: " Andy Shevchenko
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2025-03-21 14:40 UTC (permalink / raw)
  To: Andy Shevchenko, Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel
  Cc: Andy Shevchenko, Mathieu Desnoyers, Andrew Morton, Petr Mladek,
	Rasmus Villemoes, Sergey Senozhatsky

Binary printing functions are using printf() type of format, and compiler
is not happy about them as is:

kernel/trace/trace.c:3292:9: error: function ‘trace_vbprintk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
kernel/trace/trace_seq.c:182:9: error: function ‘trace_seq_bprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]

Fix the compilation errors by adding __printf() attribute.

While at it, move existing __printf() attributes from the implementations
to the declarations. IT also fixes incorrect attribute parameters that are
used for trace_array_printk().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/trace.h     |  4 ++--
 include/linux/trace_seq.h |  8 ++++----
 kernel/trace/trace.c      | 11 +++--------
 kernel/trace/trace.h      | 16 +++++++++-------
 4 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/include/linux/trace.h b/include/linux/trace.h
index fdcd76b7be83..7eaad857dee0 100644
--- a/include/linux/trace.h
+++ b/include/linux/trace.h
@@ -72,8 +72,8 @@ static inline int unregister_ftrace_export(struct trace_export *export)
 static inline void trace_printk_init_buffers(void)
 {
 }
-static inline int trace_array_printk(struct trace_array *tr, unsigned long ip,
-				     const char *fmt, ...)
+static inline __printf(3, 4)
+int trace_array_printk(struct trace_array *tr, unsigned long ip, const char *fmt, ...)
 {
 	return 0;
 }
diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h
index 1ef95c0287f0..a93ed5ac3226 100644
--- a/include/linux/trace_seq.h
+++ b/include/linux/trace_seq.h
@@ -88,8 +88,8 @@ extern __printf(2, 3)
 void trace_seq_printf(struct trace_seq *s, const char *fmt, ...);
 extern __printf(2, 0)
 void trace_seq_vprintf(struct trace_seq *s, const char *fmt, va_list args);
-extern void
-trace_seq_bprintf(struct trace_seq *s, const char *fmt, const u32 *binary);
+extern __printf(2, 0)
+void trace_seq_bprintf(struct trace_seq *s, const char *fmt, const u32 *binary);
 extern int trace_print_seq(struct seq_file *m, struct trace_seq *s);
 extern int trace_seq_to_user(struct trace_seq *s, char __user *ubuf,
 			     int cnt);
@@ -113,8 +113,8 @@ static inline __printf(2, 3)
 void trace_seq_printf(struct trace_seq *s, const char *fmt, ...)
 {
 }
-static inline void
-trace_seq_bprintf(struct trace_seq *s, const char *fmt, const u32 *binary)
+static inline __printf(2, 0)
+void trace_seq_bprintf(struct trace_seq *s, const char *fmt, const u32 *binary)
 {
 }
 
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 0e6d517e74e0..989a3f54384a 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3322,10 +3322,9 @@ int trace_vbprintk(unsigned long ip, const char *fmt, va_list args)
 }
 EXPORT_SYMBOL_GPL(trace_vbprintk);
 
-__printf(3, 0)
-static int
-__trace_array_vprintk(struct trace_buffer *buffer,
-		      unsigned long ip, const char *fmt, va_list args)
+static __printf(3, 0)
+int __trace_array_vprintk(struct trace_buffer *buffer,
+			  unsigned long ip, const char *fmt, va_list args)
 {
 	struct ring_buffer_event *event;
 	int len = 0, size;
@@ -3375,7 +3374,6 @@ __trace_array_vprintk(struct trace_buffer *buffer,
 	return len;
 }
 
-__printf(3, 0)
 int trace_array_vprintk(struct trace_array *tr,
 			unsigned long ip, const char *fmt, va_list args)
 {
@@ -3405,7 +3403,6 @@ int trace_array_vprintk(struct trace_array *tr,
  * Note, trace_array_init_printk() must be called on @tr before this
  * can be used.
  */
-__printf(3, 0)
 int trace_array_printk(struct trace_array *tr,
 		       unsigned long ip, const char *fmt, ...)
 {
@@ -3450,7 +3447,6 @@ int trace_array_init_printk(struct trace_array *tr)
 }
 EXPORT_SYMBOL_GPL(trace_array_init_printk);
 
-__printf(3, 4)
 int trace_array_printk_buf(struct trace_buffer *buffer,
 			   unsigned long ip, const char *fmt, ...)
 {
@@ -3466,7 +3462,6 @@ int trace_array_printk_buf(struct trace_buffer *buffer,
 	return ret;
 }
 
-__printf(2, 0)
 int trace_vprintk(unsigned long ip, const char *fmt, va_list args)
 {
 	return trace_array_vprintk(printk_trace, ip, fmt, args);
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 9c21ba45b7af..447d4f2a7fd2 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -836,13 +836,15 @@ static inline void __init disable_tracing_selftest(const char *reason)
 
 extern void *head_page(struct trace_array_cpu *data);
 extern unsigned long long ns2usecs(u64 nsec);
-extern int
-trace_vbprintk(unsigned long ip, const char *fmt, va_list args);
-extern int
-trace_vprintk(unsigned long ip, const char *fmt, va_list args);
-extern int
-trace_array_vprintk(struct trace_array *tr,
-		    unsigned long ip, const char *fmt, va_list args);
+
+__printf(2, 0)
+int trace_vbprintk(unsigned long ip, const char *fmt, va_list args);
+__printf(2, 0)
+int trace_vprintk(unsigned long ip, const char *fmt, va_list args);
+__printf(3, 0)
+int trace_array_vprintk(struct trace_array *tr,
+			unsigned long ip, const char *fmt, va_list args);
+__printf(3, 4)
 int trace_array_printk_buf(struct trace_buffer *buffer,
 			   unsigned long ip, const char *fmt, ...);
 void trace_printk_seq(struct trace_seq *s);
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 4/6] vsnprintf: Mark binary printing functions with __printf() attribute
  2025-03-21 14:40 [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Andy Shevchenko
                   ` (2 preceding siblings ...)
  2025-03-21 14:40 ` [PATCH v2 3/6] tracing: " Andy Shevchenko
@ 2025-03-21 14:40 ` Andy Shevchenko
  2025-03-24 19:20   ` Steven Rostedt
  2025-03-21 14:40 ` [PATCH v2 5/6] vsnprintf: Drop unused const char fmt * in va_format() Andy Shevchenko
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2025-03-21 14:40 UTC (permalink / raw)
  To: Andy Shevchenko, Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel
  Cc: Andy Shevchenko, Mathieu Desnoyers, Andrew Morton, Petr Mladek,
	Rasmus Villemoes, Sergey Senozhatsky

Binary printf() functions are using printf() type of format, and compiler
is not happy about them as is:

lib/vsprintf.c:3130:47: error: function ‘vbin_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
lib/vsprintf.c:3298:33: error: function ‘bstr_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]

Fix the compilation errors by adding __printf() attribute.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/string.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index f8e21e80942f..f15696e8e4d4 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -336,8 +336,8 @@ int __sysfs_match_string(const char * const *array, size_t n, const char *s);
 #define sysfs_match_string(_a, _s) __sysfs_match_string(_a, ARRAY_SIZE(_a), _s)
 
 #ifdef CONFIG_BINARY_PRINTF
-int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
-int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf);
+__printf(3, 0) int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
+__printf(3, 0) int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf);
 #endif
 
 extern ssize_t memory_read_from_buffer(void *to, size_t count, loff_t *ppos,
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 5/6] vsnprintf: Drop unused const char fmt * in va_format()
  2025-03-21 14:40 [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Andy Shevchenko
                   ` (3 preceding siblings ...)
  2025-03-21 14:40 ` [PATCH v2 4/6] vsnprintf: " Andy Shevchenko
@ 2025-03-21 14:40 ` Andy Shevchenko
  2025-03-21 14:40 ` [PATCH v2 6/6] vsnprintf: Silence false positive GCC warning for va_format() Andy Shevchenko
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2025-03-21 14:40 UTC (permalink / raw)
  To: Andy Shevchenko, Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel
  Cc: Andy Shevchenko, Mathieu Desnoyers, Andrew Morton, Petr Mladek,
	Rasmus Villemoes, Sergey Senozhatsky, Rasmus Villemoes

va_format() doesn't use original formatting string, drop that
argument as it's done elsewhere in similar cases.

Suggested-by: Rasmus Villemoes <ravi@prevas.dk>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/vsprintf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 56fe96319292..899e14e4c469 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1693,7 +1693,7 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
 }
 
 static char *va_format(char *buf, char *end, struct va_format *va_fmt,
-		       struct printf_spec spec, const char *fmt)
+		       struct printf_spec spec)
 {
 	va_list va;
 
@@ -2462,7 +2462,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr,
 	case 'U':
 		return uuid_string(buf, end, ptr, spec, fmt);
 	case 'V':
-		return va_format(buf, end, ptr, spec, fmt);
+		return va_format(buf, end, ptr, spec);
 	case 'K':
 		return restricted_pointer(buf, end, ptr, spec);
 	case 'N':
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v2 6/6] vsnprintf: Silence false positive GCC warning for va_format()
  2025-03-21 14:40 [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Andy Shevchenko
                   ` (4 preceding siblings ...)
  2025-03-21 14:40 ` [PATCH v2 5/6] vsnprintf: Drop unused const char fmt * in va_format() Andy Shevchenko
@ 2025-03-21 14:40 ` Andy Shevchenko
  2025-03-25 10:15 ` [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Petr Mladek
  2025-03-25 19:38 ` Kees Cook
  7 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2025-03-21 14:40 UTC (permalink / raw)
  To: Andy Shevchenko, Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel
  Cc: Andy Shevchenko, Mathieu Desnoyers, Andrew Morton, Petr Mladek,
	Rasmus Villemoes, Sergey Senozhatsky, Rasmus Villemoes

va_format() is using vsnprintf(), and GCC compiler (Debian 14.2.0-17)
is not happy about this:

lib/vsprintf.c:1704:9: error: function ‘va_format’ might be a candidate for ‘gnu_print ’ format attribute [-Werror=suggest-attribute=format]

Fix the compilation errors (`make W=1` when CONFIG_WERROR=y, which is default)
by silencing the false positive GCC warning.

Suggested-by: Rasmus Villemoes <ravi@prevas.dk>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/vsprintf.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 899e14e4c469..a55e88895a64 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1692,6 +1692,10 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
 	return buf;
 }
 
+#pragma GCC diagnostic push
+#ifndef __clang__
+#pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
+#endif
 static char *va_format(char *buf, char *end, struct va_format *va_fmt,
 		       struct printf_spec spec)
 {
@@ -1706,6 +1710,7 @@ static char *va_format(char *buf, char *end, struct va_format *va_fmt,
 
 	return buf;
 }
+#pragma GCC diagnostic pop
 
 static noinline_for_stack
 char *uuid_string(char *buf, char *end, const u8 *addr,
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 4/6] vsnprintf: Mark binary printing functions with __printf() attribute
  2025-03-21 14:40 ` [PATCH v2 4/6] vsnprintf: " Andy Shevchenko
@ 2025-03-24 19:20   ` Steven Rostedt
  2025-03-24 19:32     ` Andy Shevchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Steven Rostedt @ 2025-03-24 19:20 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Christophe JAILLET, Kees Cook, Masami Hiramatsu (Google),
	linux-kernel, linux-hardening, linux-trace-kernel,
	Andy Shevchenko, Mathieu Desnoyers, Andrew Morton, Petr Mladek,
	Rasmus Villemoes, Sergey Senozhatsky

On Fri, 21 Mar 2025 16:40:50 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> Binary printf() functions are using printf() type of format, and compiler
> is not happy about them as is:
> 
> lib/vsprintf.c:3130:47: error: function ‘vbin_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> lib/vsprintf.c:3298:33: error: function ‘bstr_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]

BTW, I find it disturbing that the compiler is set to "error" on a warning
that "might be a candidate". What happens if it is not? We have to play
games to quiet it.

Adding __printf() attributes to stubs seems to be a case of the compiler
causing more problems than its worth :-/

I honestly hate this error on warning because it causes real pain when
debugging. There's a lot of times I don't know if the value is long or long
long, and when I get it wrong, my printk() causes the build to fail. It's
especially annoying when both long and long long are the same size!

Fixing theses stupid errors takes a non trivial amount of time away from
actual debugging.

-- Steve


> 
> Fix the compilation errors by adding __printf() attribute.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 4/6] vsnprintf: Mark binary printing functions with __printf() attribute
  2025-03-24 19:20   ` Steven Rostedt
@ 2025-03-24 19:32     ` Andy Shevchenko
  0 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2025-03-24 19:32 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Christophe JAILLET, Kees Cook, Masami Hiramatsu (Google),
	linux-kernel, linux-hardening, linux-trace-kernel,
	Mathieu Desnoyers, Andrew Morton, Petr Mladek, Rasmus Villemoes,
	Sergey Senozhatsky

On Mon, Mar 24, 2025 at 03:20:12PM -0400, Steven Rostedt wrote:
> On Fri, 21 Mar 2025 16:40:50 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > Binary printf() functions are using printf() type of format, and compiler
> > is not happy about them as is:
> > 
> > lib/vsprintf.c:3130:47: error: function ‘vbin_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> > lib/vsprintf.c:3298:33: error: function ‘bstr_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
> 
> BTW, I find it disturbing that the compiler is set to "error" on a warning
> that "might be a candidate". What happens if it is not? We have to play
> games to quiet it.
> 
> Adding __printf() attributes to stubs seems to be a case of the compiler
> causing more problems than its worth :-/
> 
> I honestly hate this error on warning because it causes real pain when
> debugging. 

Tell it to Linus :-) since it was him who enabled that default. And since it's
there and defconfigs are also part of the kernel I can't easy remove that, and
TBH I even won't dare doing that.

> There's a lot of times I don't know if the value is long or long
> long, and when I get it wrong, my printk() causes the build to fail. It's
> especially annoying when both long and long long are the same size!
> 
> Fixing theses stupid errors takes a non trivial amount of time away from
> actual debugging.

You (actually me) fix them once, currently CI's typically run with W=1, but
with WERROR=n. Which means that the new code that is not fixed a priori, will
induce the CI red report.

> > Fix the compilation errors by adding __printf() attribute.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required
  2025-03-21 14:40 [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Andy Shevchenko
                   ` (5 preceding siblings ...)
  2025-03-21 14:40 ` [PATCH v2 6/6] vsnprintf: Silence false positive GCC warning for va_format() Andy Shevchenko
@ 2025-03-25 10:15 ` Petr Mladek
  2025-03-28 13:51   ` Petr Mladek
  2025-03-25 19:38 ` Kees Cook
  7 siblings, 1 reply; 13+ messages in thread
From: Petr Mladek @ 2025-03-25 10:15 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel, Andy Shevchenko, Mathieu Desnoyers,
	Andrew Morton, Rasmus Villemoes, Sergey Senozhatsky

On Fri 2025-03-21 16:40:46, Andy Shevchenko wrote:
> This whole series started from a simple fix (see the last patch)
> to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
> (note, that CONFIG_WERROR=y and all warnings break the build!)
> down to a rabbit hole.
> 
> However starting from v2 the last patch doesn't require the first
> part, I prefer still to have them since the functions, while being
> _binary_ printf()-like, are still printf()-like. It also puts in align
> the tracing stuff with the rest and fixes the wrong parameter value.
> 
> These first 4 patches are organised in a strict order and can't be
> reshuffled, otherwise it will produce a warnings in between.
> 
> I believe the best route for the series is printk tree with immutable
> tag or branch for the others.
> 
> Alternatively the first 4 patches can be applied first as they
> are pretty much straightforward. They also can be squashed to one
> (as the same topic behind), but it all is up to the respective
> maintainers.

The whole series looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

I am going to push it via the printk tree. I think about doing
so as a second pull request by the end of this merge window.

Anyway, I am going to wait few more days for eventual feedback
or push back.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required
  2025-03-21 14:40 [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Andy Shevchenko
                   ` (6 preceding siblings ...)
  2025-03-25 10:15 ` [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Petr Mladek
@ 2025-03-25 19:38 ` Kees Cook
  7 siblings, 0 replies; 13+ messages in thread
From: Kees Cook @ 2025-03-25 19:38 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Christophe JAILLET, Steven Rostedt, Masami Hiramatsu (Google),
	linux-kernel, linux-hardening, linux-trace-kernel,
	Andy Shevchenko, Mathieu Desnoyers, Andrew Morton, Petr Mladek,
	Rasmus Villemoes, Sergey Senozhatsky

On Fri, Mar 21, 2025 at 04:40:46PM +0200, Andy Shevchenko wrote:
> This whole series started from a simple fix (see the last patch)
> to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
> (note, that CONFIG_WERROR=y and all warnings break the build!)
> down to a rabbit hole.
> 
> However starting from v2 the last patch doesn't require the first
> part, I prefer still to have them since the functions, while being
> _binary_ printf()-like, are still printf()-like. It also puts in align
> the tracing stuff with the rest and fixes the wrong parameter value.
> 
> These first 4 patches are organised in a strict order and can't be
> reshuffled, otherwise it will produce a warnings in between.
> 
> I believe the best route for the series is printk tree with immutable
> tag or branch for the others.
> 
> Alternatively the first 4 patches can be applied first as they
> are pretty much straightforward. They also can be squashed to one
> (as the same topic behind), but it all is up to the respective
> maintainers.
> 
> In v2:
> - split out patch 5 (Rasmus)
> - rewritten the approach for the va_format() fix (Rasmus)
> - amended tracing patch (removed a blank line and a __printf() in C file)
> 
> Andy Shevchenko (6):
>   seq_buf: Mark binary printing functions with __printf() attribute
>   seq_file: Mark binary printing functions with __printf() attribute
>   tracing: Mark binary printing functions with __printf() attribute
>   vsnprintf: Mark binary printing functions with __printf() attribute
>   vsnprintf: Drop unused const char fmt * in va_format()
>   vsnprintf: Silence false positive GCC warning for va_format()
> 
>  include/linux/seq_buf.h   |  4 ++--
>  include/linux/seq_file.h  |  1 +
>  include/linux/string.h    |  4 ++--
>  include/linux/trace.h     |  4 ++--
>  include/linux/trace_seq.h |  8 ++++----
>  kernel/trace/trace.c      | 11 +++--------
>  kernel/trace/trace.h      | 16 +++++++++-------
>  lib/vsprintf.c            |  9 +++++++--
>  8 files changed, 30 insertions(+), 27 deletions(-)

Cool; it'll be nice to get these marked up.

Reviewed-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required
  2025-03-25 10:15 ` [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Petr Mladek
@ 2025-03-28 13:51   ` Petr Mladek
  2025-03-28 15:05     ` Andy Shevchenko
  0 siblings, 1 reply; 13+ messages in thread
From: Petr Mladek @ 2025-03-28 13:51 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel, Andy Shevchenko, Mathieu Desnoyers,
	Andrew Morton, Rasmus Villemoes, Sergey Senozhatsky

On Tue 2025-03-25 11:15:21, Petr Mladek wrote:
> On Fri 2025-03-21 16:40:46, Andy Shevchenko wrote:
> > This whole series started from a simple fix (see the last patch)
> > to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
> > (note, that CONFIG_WERROR=y and all warnings break the build!)
> > down to a rabbit hole.
> > 
> > However starting from v2 the last patch doesn't require the first
> > part, I prefer still to have them since the functions, while being
> > _binary_ printf()-like, are still printf()-like. It also puts in align
> > the tracing stuff with the rest and fixes the wrong parameter value.
> > 
> > These first 4 patches are organised in a strict order and can't be
> > reshuffled, otherwise it will produce a warnings in between.
> > 
> > I believe the best route for the series is printk tree with immutable
> > tag or branch for the others.
> > 
> > Alternatively the first 4 patches can be applied first as they
> > are pretty much straightforward. They also can be squashed to one
> > (as the same topic behind), but it all is up to the respective
> > maintainers.
> 
> The whole series looks good to me:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> 
> I am going to push it via the printk tree. I think about doing
> so as a second pull request by the end of this merge window.
> 
> Anyway, I am going to wait few more days for eventual feedback
> or push back.

JFYI, I have pushed the patchset into printk/linux.git,
branch for-6.15-printf-attribute.

I am going to send a pull request the following week
if nothing happens in the meantime.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required
  2025-03-28 13:51   ` Petr Mladek
@ 2025-03-28 15:05     ` Andy Shevchenko
  0 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2025-03-28 15:05 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Andy Shevchenko, Christophe JAILLET, Kees Cook, Steven Rostedt,
	Masami Hiramatsu (Google), linux-kernel, linux-hardening,
	linux-trace-kernel, Andy Shevchenko, Mathieu Desnoyers,
	Andrew Morton, Rasmus Villemoes, Sergey Senozhatsky

On Fri, Mar 28, 2025 at 3:51 PM Petr Mladek <pmladek@suse.com> wrote:
> On Tue 2025-03-25 11:15:21, Petr Mladek wrote:
> > On Fri 2025-03-21 16:40:46, Andy Shevchenko wrote:
> > > This whole series started from a simple fix (see the last patch)
> > > to make GCC (Debian 14.2.0-17) happy when compiling with `make W=1`
> > > (note, that CONFIG_WERROR=y and all warnings break the build!)
> > > down to a rabbit hole.
> > >
> > > However starting from v2 the last patch doesn't require the first
> > > part, I prefer still to have them since the functions, while being
> > > _binary_ printf()-like, are still printf()-like. It also puts in align
> > > the tracing stuff with the rest and fixes the wrong parameter value.
> > >
> > > These first 4 patches are organised in a strict order and can't be
> > > reshuffled, otherwise it will produce a warnings in between.
> > >
> > > I believe the best route for the series is printk tree with immutable
> > > tag or branch for the others.
> > >
> > > Alternatively the first 4 patches can be applied first as they
> > > are pretty much straightforward. They also can be squashed to one
> > > (as the same topic behind), but it all is up to the respective
> > > maintainers.
> >
> > The whole series looks good to me:
> >
> > Reviewed-by: Petr Mladek <pmladek@suse.com>
> >
> > I am going to push it via the printk tree. I think about doing
> > so as a second pull request by the end of this merge window.
> >
> > Anyway, I am going to wait few more days for eventual feedback
> > or push back.
>
> JFYI, I have pushed the patchset into printk/linux.git,
> branch for-6.15-printf-attribute.
>
> I am going to send a pull request the following week
> if nothing happens in the meantime.

Thank you!
It will make us much closer to the moment when we can enable WERROR=y
in the CIs for `make W=1` on x86 defconfigs (those that are in the
kernel source tree).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-03-28 15:06 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-21 14:40 [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Andy Shevchenko
2025-03-21 14:40 ` [PATCH v2 1/6] seq_buf: Mark binary printing functions with __printf() attribute Andy Shevchenko
2025-03-21 14:40 ` [PATCH v2 2/6] seq_file: " Andy Shevchenko
2025-03-21 14:40 ` [PATCH v2 3/6] tracing: " Andy Shevchenko
2025-03-21 14:40 ` [PATCH v2 4/6] vsnprintf: " Andy Shevchenko
2025-03-24 19:20   ` Steven Rostedt
2025-03-24 19:32     ` Andy Shevchenko
2025-03-21 14:40 ` [PATCH v2 5/6] vsnprintf: Drop unused const char fmt * in va_format() Andy Shevchenko
2025-03-21 14:40 ` [PATCH v2 6/6] vsnprintf: Silence false positive GCC warning for va_format() Andy Shevchenko
2025-03-25 10:15 ` [PATCH v2 0/6] vsprintf: Add __printf attribute to where it's required Petr Mladek
2025-03-28 13:51   ` Petr Mladek
2025-03-28 15:05     ` Andy Shevchenko
2025-03-25 19:38 ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).