diff mbox series

net: l2tp: fix clang -Wformat warning

Message ID 20220706230833.535238-1-justinstitt@google.com (mailing list archive)
State Accepted
Commit a2b6111b55f3d1b8d303e986db9d761571793aba
Delegated to: Netdev Maintainers
Headers show
Series net: l2tp: fix clang -Wformat warning | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix warning Target tree name not specified in the subject
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Justin Stitt July 6, 2022, 11:08 p.m. UTC
When building with clang we encounter this warning:
| net/l2tp/l2tp_ppp.c:1557:6: error: format specifies type 'unsigned
| short' but the argument has type 'u32' (aka 'unsigned int')
| [-Werror,-Wformat] session->nr, session->ns,

Both session->nr and session->ns are of type u32. The format specifier
previously used is `%hu` which would truncate our unsigned integer from
32 to 16 bits. This doesn't seem like intended behavior, if it is then
perhaps we need to consider suppressing the warning with pragma clauses.

This patch should get us closer to the goal of enabling the -Wformat
flag for Clang builds.

Link: https://github.com/ClangBuiltLinux/linux/issues/378
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
 net/l2tp/l2tp_ppp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Guillaume Nault July 7, 2022, 7:50 a.m. UTC | #1
On Wed, Jul 06, 2022 at 04:08:33PM -0700, Justin Stitt wrote:
> When building with clang we encounter this warning:
> | net/l2tp/l2tp_ppp.c:1557:6: error: format specifies type 'unsigned
> | short' but the argument has type 'u32' (aka 'unsigned int')
> | [-Werror,-Wformat] session->nr, session->ns,
> 
> Both session->nr and session->ns are of type u32. The format specifier
> previously used is `%hu` which would truncate our unsigned integer from
> 32 to 16 bits. This doesn't seem like intended behavior, if it is then
> perhaps we need to consider suppressing the warning with pragma clauses.

pppol2tp_seq_session_show() is only called for L2TPv2 sessions, where
ns and nr are 2 bytes long (L2TPv3 uses 3 bytes, hence the u32 type in
the generic l2tp_session structure). So %hu shouldn't truncate anything
here.

However %u doesn't harm and is cleaner than silencing the warning with
pragma.

Acked-by: Guillaume Nault <gnault@redhat.com>


> This patch should get us closer to the goal of enabling the -Wformat
> flag for Clang builds.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
>  net/l2tp/l2tp_ppp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
> index 8be1fdc68a0b..db2e584c625e 100644
> --- a/net/l2tp/l2tp_ppp.c
> +++ b/net/l2tp/l2tp_ppp.c
> @@ -1553,7 +1553,7 @@ static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
>  		   session->lns_mode ? "LNS" : "LAC",
>  		   0,
>  		   jiffies_to_msecs(session->reorder_timeout));
> -	seq_printf(m, "   %hu/%hu %ld/%ld/%ld %ld/%ld/%ld\n",
> +	seq_printf(m, "   %u/%u %ld/%ld/%ld %ld/%ld/%ld\n",
>  		   session->nr, session->ns,
>  		   atomic_long_read(&session->stats.tx_packets),
>  		   atomic_long_read(&session->stats.tx_bytes),
> -- 
> 2.37.0.rc0.161.g10f37bed90-goog
>
patchwork-bot+netdevbpf@kernel.org July 8, 2022, 1:30 a.m. UTC | #2
Hello:

This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  6 Jul 2022 16:08:33 -0700 you wrote:
> When building with clang we encounter this warning:
> | net/l2tp/l2tp_ppp.c:1557:6: error: format specifies type 'unsigned
> | short' but the argument has type 'u32' (aka 'unsigned int')
> | [-Werror,-Wformat] session->nr, session->ns,
> 
> Both session->nr and session->ns are of type u32. The format specifier
> previously used is `%hu` which would truncate our unsigned integer from
> 32 to 16 bits. This doesn't seem like intended behavior, if it is then
> perhaps we need to consider suppressing the warning with pragma clauses.
> 
> [...]

Here is the summary with links:
  - net: l2tp: fix clang -Wformat warning
    https://git.kernel.org/netdev/net-next/c/a2b6111b55f3

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index 8be1fdc68a0b..db2e584c625e 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1553,7 +1553,7 @@  static void pppol2tp_seq_session_show(struct seq_file *m, void *v)
 		   session->lns_mode ? "LNS" : "LAC",
 		   0,
 		   jiffies_to_msecs(session->reorder_timeout));
-	seq_printf(m, "   %hu/%hu %ld/%ld/%ld %ld/%ld/%ld\n",
+	seq_printf(m, "   %u/%u %ld/%ld/%ld %ld/%ld/%ld\n",
 		   session->nr, session->ns,
 		   atomic_long_read(&session->stats.tx_packets),
 		   atomic_long_read(&session->stats.tx_bytes),