[bpf] bpf: annotate BTF show functions with __printf

Commit Message

Alan Maguire July 11, 2024, 6:23 p.m. UTC

-Werror=suggest-attribute=format warns about two functions
in kernel/bpf/btf.c [1]; add __printf() annotations to silence
these warnings since for CONFIG_WERROR=y they will trigger
build failures.

[1] https://lore.kernel.org/bpf/[e-mail]/

Fixes: 31d0bc81637d ("bpf: Move to generic BTF show support, apply it to seq files/strings")
Reported-by: Mirsad Todorovac <[e-mail]>
Signed-off-by: Alan Maguire <[e-mail]>
---
 kernel/bpf/btf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Mirsad Todorovac July 11, 2024, 9:10 p.m. UTC | #1

On 7/11/24 20:23, Alan Maguire wrote:
> -Werror=suggest-attribute=format warns about two functions
> in kernel/bpf/btf.c [1]; add __printf() annotations to silence
> these warnings since for CONFIG_WERROR=y they will trigger
> build failures.
> 
> [1] https://lore.kernel.org/bpf/[e-mail]/
> 
> Fixes: 31d0bc81637d ("bpf: Move to generic BTF show support, apply it to seq files/strings")
> Reported-by: Mirsad Todorovac <[e-mail]>
> Signed-off-by: Alan Maguire <[e-mail]>
> ---
>  kernel/bpf/btf.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 4ff11779699e..d5019c4454d6 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7538,8 +7538,8 @@ static void btf_type_show(const struct btf *btf, u32 type_id, void *obj,
>  	btf_type_ops(t)->show(btf, t, type_id, obj, 0, show);
>  }
>  
> -static void btf_seq_show(struct btf_show *show, const char *fmt,
> -			 va_list args)
> +__printf(2, 0) static void btf_seq_show(struct btf_show *show, const char *fmt,
> +					va_list args)
>  {
>  	seq_vprintf((struct seq_file *)show->target, fmt, args);
>  }
> @@ -7572,8 +7572,8 @@ struct btf_show_snprintf {
>  	int len;		/* length we would have written */
>  };
>  
> -static void btf_snprintf_show(struct btf_show *show, const char *fmt,
> -			      va_list args)
> +__printf(2, 0) static void btf_snprintf_show(struct btf_show *show, const char *fmt,
> +					     va_list args)
>  {
>  	struct btf_show_snprintf *ssnprintf = (struct btf_show_snprintf *)show;
>  	int len;

Hi,

The module compiled without warnings/errors in the same testing environment:

  CC      kernel/bpf/btf.o

Tested-by: Mirsad Todorovac <[e-mail]>

Best regards,
Mirsad Todorovac

[e-mail] July 11, 2024, 9:20 p.m. UTC | #2

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Thu, 11 Jul 2024 19:23:21 +0100 you wrote:
> -Werror=suggest-attribute=format warns about two functions
> in kernel/bpf/btf.c [1]; add __printf() annotations to silence
> these warnings since for CONFIG_WERROR=y they will trigger
> build failures.
> 
> [1] https://lore.kernel.org/bpf/[e-mail]/
> 
> [...]

Here is the summary with links:
  - [bpf] bpf: annotate BTF show functions with __printf
    https://git.kernel.org/bpf/bpf-next/c/b3470da314fd

You are awesome, thank you!

Mirsad Todorovac July 11, 2024, 9:29 p.m. UTC | #3

On 7/11/24 20:23, Alan Maguire wrote:
> -Werror=suggest-attribute=format warns about two functions
> in kernel/bpf/btf.c [1]; add __printf() annotations to silence
> these warnings since for CONFIG_WERROR=y they will trigger
> build failures.
> 
> [1] https://lore.kernel.org/bpf/[e-mail]/
> 
> Fixes: 31d0bc81637d ("bpf: Move to generic BTF show support, apply it to seq files/strings")
> Reported-by: Mirsad Todorovac <[e-mail]>
> Signed-off-by: Alan Maguire <[e-mail]>
> ---
>  kernel/bpf/btf.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 4ff11779699e..d5019c4454d6 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7538,8 +7538,8 @@ static void btf_type_show(const struct btf *btf, u32 type_id, void *obj,
>  	btf_type_ops(t)->show(btf, t, type_id, obj, 0, show);
>  }
>  
> -static void btf_seq_show(struct btf_show *show, const char *fmt,
> -			 va_list args)
> +__printf(2, 0) static void btf_seq_show(struct btf_show *show, const char *fmt,
> +					va_list args)
>  {
>  	seq_vprintf((struct seq_file *)show->target, fmt, args);
>  }
> @@ -7572,8 +7572,8 @@ struct btf_show_snprintf {
>  	int len;		/* length we would have written */
>  };
>  
> -static void btf_snprintf_show(struct btf_show *show, const char *fmt,
> -			      va_list args)
> +__printf(2, 0) static void btf_snprintf_show(struct btf_show *show, const char *fmt,
> +					     va_list args)
>  {
>  	struct btf_show_snprintf *ssnprintf = (struct btf_show_snprintf *)show;
>  	int len;

Hi,

Unfortunately, there seems to be a side-effect to your patch:

./kernel/bpf/btf.c: In function ‘btf_type_seq_show_flags’:
./kernel/bpf/btf.c:7553:21: error: assignment left-hand side might be a candidate for a format attribute [-Werror=suggest-attribute=format]
 7553 |         sseq.showfn = btf_seq_show;
      |                     ^
./kernel/bpf/btf.c: In function ‘btf_type_snprintf_show’:
./kernel/bpf/btf.c:7604:31: error: assignment left-hand side might be a candidate for a format attribute [-Werror=suggest-attribute=format]
 7604 |         ssnprintf.show.showfn = btf_snprintf_show;
      |                               ^

Thanks.

Best regards,
Mirsad Todorovac

Mirsad Todorovac July 11, 2024, 9:40 p.m. UTC | #4

Hi,

On 7/11/24 20:23, Alan Maguire wrote:
> -Werror=suggest-attribute=format warns about two functions
> in kernel/bpf/btf.c [1]; add __printf() annotations to silence
> these warnings since for CONFIG_WERROR=y they will trigger
> build failures.
> 
> [1] https://lore.kernel.org/bpf/[e-mail]/
> 
> Fixes: 31d0bc81637d ("bpf: Move to generic BTF show support, apply it to seq files/strings")
> Reported-by: Mirsad Todorovac <[e-mail]>
> Signed-off-by: Alan Maguire <[e-mail]>
> ---
>  kernel/bpf/btf.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 4ff11779699e..d5019c4454d6 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7538,8 +7538,8 @@ static void btf_type_show(const struct btf *btf, u32 type_id, void *obj,
>  	btf_type_ops(t)->show(btf, t, type_id, obj, 0, show);
>  }
>  
> -static void btf_seq_show(struct btf_show *show, const char *fmt,
> -			 va_list args)
> +__printf(2, 0) static void btf_seq_show(struct btf_show *show, const char *fmt,
> +					va_list args)
>  {
>  	seq_vprintf((struct seq_file *)show->target, fmt, args);
>  }
> @@ -7572,8 +7572,8 @@ struct btf_show_snprintf {
>  	int len;		/* length we would have written */
>  };
>  
> -static void btf_snprintf_show(struct btf_show *show, const char *fmt,
> -			      va_list args)
> +__printf(2, 0) static void btf_snprintf_show(struct btf_show *show, const char *fmt,
> +					     va_list args)
>  {
>  	struct btf_show_snprintf *ssnprintf = (struct btf_show_snprintf *)show;
>  	int len;

The error returned from your patch is here:

./kernel/bpf/btf.c: In function ‘btf_type_seq_show_flags’:
./kernel/bpf/btf.c:7553:21: error: assignment left-hand side might be a candidate for a format attribute [-Werror=suggest-attribute=format]
 7553 |         sseq.showfn = btf_seq_show;
      |                     ^
./kernel/bpf/btf.c: In function ‘btf_type_snprintf_show’:
./kernel/bpf/btf.c:7604:31: error: assignment left-hand side might be a candidate for a format attribute [-Werror=suggest-attribute=format]
 7604 |         ssnprintf.show.showfn = btf_snprintf_show;
      |                               ^

The patch is quite straightforward:

-----------------------><-------------------------------
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 4ff11779699e..9711ee6a31e5 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -415,7 +415,7 @@ const char *btf_type_str(const struct btf_type *t)
 struct btf_show {
        u64 flags;
        void *target;   /* target of show operation (seq file, buffer) */
-       void (*showfn)(struct btf_show *show, const char *fmt, va_list args);
+       __printf(2, 0) void (*showfn)(struct btf_show *show, const char *fmt, va_list args);
        const struct btf *btf;
        /* below are used during iteration */
        struct {
--


Life long and prosper!

Best regards,
Mirsad Todorovac

Patch

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 4ff11779699e..d5019c4454d6 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7538,8 +7538,8 @@  static void btf_type_show(const struct btf *btf, u32 type_id, void *obj,
 	btf_type_ops(t)->show(btf, t, type_id, obj, 0, show);
 }
 
-static void btf_seq_show(struct btf_show *show, const char *fmt,
-			 va_list args)
+__printf(2, 0) static void btf_seq_show(struct btf_show *show, const char *fmt,
+					va_list args)
 {
 	seq_vprintf((struct seq_file *)show->target, fmt, args);
 }
@@ -7572,8 +7572,8 @@  struct btf_show_snprintf {
 	int len;		/* length we would have written */
 };
 
-static void btf_snprintf_show(struct btf_show *show, const char *fmt,
-			      va_list args)
+__printf(2, 0) static void btf_snprintf_show(struct btf_show *show, const char *fmt,
+					     va_list args)
 {
 	struct btf_show_snprintf *ssnprintf = (struct btf_show_snprintf *)show;
 	int len;