alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] soc: sof: fix clang -Wformat warnings
@ 2022-07-21 21:12 Justin Stitt
  2022-07-21 21:22 ` Nathan Chancellor
  2022-08-02 20:25 ` Nathan Chancellor
  0 siblings, 2 replies; 4+ messages in thread
From: Justin Stitt @ 2022-07-21 21:12 UTC (permalink / raw)
  To: Pierre-Louis Bossart, Liam Girdwood, Peter Ujfalusi, Bard Liao,
	Ranjani Sridharan, Daniel Baluta, Mark Brown, Jaroslav Kysela,
	Takashi Iwai, Nathan Chancellor, Nick Desaulniers
  Cc: alsa-devel, Kai Vehmanen, Tom Rix, llvm, linux-kernel,
	Justin Stitt, sound-open-firmware

When building with Clang we encounter these warnings:
| sound/soc/sof/ipc3-topology.c:2343:4: error: format specifies type
| 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
|                  SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
|                  ^~~~~~~~~~~~~~~^~~~~~~~~~~~~~~^~~~~~~~~~~~~

Use correct format specifier `%d` since args are of type int.

Link: https://github.com/ClangBuiltLinux/linux/issues/378
Reported-by: Nathan Chancellor <nathan@kernel.org>
Suggested-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
---
Reported by Nathan here:
https://lore.kernel.org/all/YtmrCJjQrSbv8Aj1@dev-arch.thelio-3990X/

 sound/soc/sof/ipc3-topology.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
index b2cc046b9f60..65923e7a5976 100644
--- a/sound/soc/sof/ipc3-topology.c
+++ b/sound/soc/sof/ipc3-topology.c
@@ -2338,7 +2338,7 @@ static int sof_ipc3_parse_manifest(struct snd_soc_component *scomp, int index,
 	}
 
 	dev_info(scomp->dev,
-		 "Topology: ABI %d:%d:%d Kernel ABI %hhu:%hhu:%hhu\n",
+		 "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n",
 		 man->priv.data[0], man->priv.data[1], man->priv.data[2],
 		 SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
 
-- 
2.37.1.359.gd136c6c3e2-goog


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

* Re: [PATCH] soc: sof: fix clang -Wformat warnings
  2022-07-21 21:12 [PATCH] soc: sof: fix clang -Wformat warnings Justin Stitt
@ 2022-07-21 21:22 ` Nathan Chancellor
  2022-08-02 20:25 ` Nathan Chancellor
  1 sibling, 0 replies; 4+ messages in thread
From: Nathan Chancellor @ 2022-07-21 21:22 UTC (permalink / raw)
  To: Justin Stitt
  Cc: Nick Desaulniers, alsa-devel, llvm, Kai Vehmanen, Liam Girdwood,
	Bard Liao, Takashi Iwai, Peter Ujfalusi, Ranjani Sridharan,
	Pierre-Louis Bossart, Mark Brown, Tom Rix, Daniel Baluta,
	linux-kernel, sound-open-firmware

On Thu, Jul 21, 2022 at 02:12:18PM -0700, Justin Stitt wrote:
> When building with Clang we encounter these warnings:
> | sound/soc/sof/ipc3-topology.c:2343:4: error: format specifies type
> | 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
> |                  SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
> |                  ^~~~~~~~~~~~~~~^~~~~~~~~~~~~~~^~~~~~~~~~~~~
> 
> Use correct format specifier `%d` since args are of type int.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Justin Stitt <justinstitt@google.com>

Indeed, decimal integer literals with no suffix are of type 'int' when
they can fit in an 'int'. In this case, there shouldn't be a bug since
the values of these macros can fit in an 'unsigned char' (so no
truncation) but it is still correct to use '%d' instead of '%hhu', which
matches the stance of commit cbacb5ab0aa0 ("docs: printk-formats: Stop
encouraging use of unnecessary %h[xudi] and %hh[xudi]").

This was introduced by commit 323aa1f093e6 ("ASoC: SOF: Add a new IPC op
for parsing topology manifest"), not sure it warrants a fixes tag for
the reason I outlined above, but it might be helpful for other
reviewers.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> Reported by Nathan here:
> https://lore.kernel.org/all/YtmrCJjQrSbv8Aj1@dev-arch.thelio-3990X/
> 
>  sound/soc/sof/ipc3-topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
> index b2cc046b9f60..65923e7a5976 100644
> --- a/sound/soc/sof/ipc3-topology.c
> +++ b/sound/soc/sof/ipc3-topology.c
> @@ -2338,7 +2338,7 @@ static int sof_ipc3_parse_manifest(struct snd_soc_component *scomp, int index,
>  	}
>  
>  	dev_info(scomp->dev,
> -		 "Topology: ABI %d:%d:%d Kernel ABI %hhu:%hhu:%hhu\n",
> +		 "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n",
>  		 man->priv.data[0], man->priv.data[1], man->priv.data[2],
>  		 SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
>  
> -- 
> 2.37.1.359.gd136c6c3e2-goog
> 

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

* Re: [PATCH] soc: sof: fix clang -Wformat warnings
  2022-07-21 21:12 [PATCH] soc: sof: fix clang -Wformat warnings Justin Stitt
  2022-07-21 21:22 ` Nathan Chancellor
@ 2022-08-02 20:25 ` Nathan Chancellor
  2022-08-03 11:44   ` Mark Brown
  1 sibling, 1 reply; 4+ messages in thread
From: Nathan Chancellor @ 2022-08-02 20:25 UTC (permalink / raw)
  To: Justin Stitt
  Cc: Nick Desaulniers, alsa-devel, llvm, Kai Vehmanen, Liam Girdwood,
	Bard Liao, Takashi Iwai, Peter Ujfalusi, Ranjani Sridharan,
	Pierre-Louis Bossart, Mark Brown, Tom Rix, Daniel Baluta,
	linux-kernel, sound-open-firmware

Hi all,

On Thu, Jul 21, 2022 at 02:12:18PM -0700, Justin Stitt wrote:
> When building with Clang we encounter these warnings:
> | sound/soc/sof/ipc3-topology.c:2343:4: error: format specifies type
> | 'unsigned char' but the argument has type 'int' [-Werror,-Wformat]
> |                  SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
> |                  ^~~~~~~~~~~~~~~^~~~~~~~~~~~~~~^~~~~~~~~~~~~
> 
> Use correct format specifier `%d` since args are of type int.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Signed-off-by: Justin Stitt <justinstitt@google.com>
> ---
> Reported by Nathan here:
> https://lore.kernel.org/all/YtmrCJjQrSbv8Aj1@dev-arch.thelio-3990X/
> 
>  sound/soc/sof/ipc3-topology.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/soc/sof/ipc3-topology.c b/sound/soc/sof/ipc3-topology.c
> index b2cc046b9f60..65923e7a5976 100644
> --- a/sound/soc/sof/ipc3-topology.c
> +++ b/sound/soc/sof/ipc3-topology.c
> @@ -2338,7 +2338,7 @@ static int sof_ipc3_parse_manifest(struct snd_soc_component *scomp, int index,
>  	}
>  
>  	dev_info(scomp->dev,
> -		 "Topology: ABI %d:%d:%d Kernel ABI %hhu:%hhu:%hhu\n",
> +		 "Topology: ABI %d:%d:%d Kernel ABI %d:%d:%d\n",
>  		 man->priv.data[0], man->priv.data[1], man->priv.data[2],
>  		 SOF_ABI_MAJOR, SOF_ABI_MINOR, SOF_ABI_PATCH);
>  
> -- 
> 2.37.1.359.gd136c6c3e2-goog
> 
> 

Is it too late for this patch to make 6.0? We are trying to enable
-Wformat for clang in 6.0 and this instance of that warning was
introduced this development cycle by commit 323aa1f093e6 ("ASoC: SOF:
Add a new IPC op for parsing topology manifest"). If I am tracking all
my patches correctly, this is the only instance of -Wformat that does
not have a patch applied to a maintainer's tree so it would be really
unfortunate if we could not sure it on for -rc1.

We could probably route this via the Kbuild tree with an Ack along with
the patch that enables -Wformat if it cannot go via Mark's or Takashi's
ree.

Cheers,
Nathan

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

* Re: [PATCH] soc: sof: fix clang -Wformat warnings
  2022-08-02 20:25 ` Nathan Chancellor
@ 2022-08-03 11:44   ` Mark Brown
  0 siblings, 0 replies; 4+ messages in thread
From: Mark Brown @ 2022-08-03 11:44 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Daniel Baluta, Pierre-Louis Bossart, alsa-devel, llvm,
	Kai Vehmanen, Bard Liao, Takashi Iwai, Nick Desaulniers,
	Liam Girdwood, Ranjani Sridharan, Tom Rix, Justin Stitt,
	Peter Ujfalusi, ardb, linux-kernel, sound-open-firmware

[-- Attachment #1: Type: text/plain, Size: 1563 bytes --]

On Tue, Aug 02, 2022 at 01:25:24PM -0700, Nathan Chancellor wrote:

> Is it too late for this patch to make 6.0? We are trying to enable
> -Wformat for clang in 6.0 and this instance of that warning was
> introduced this development cycle by commit 323aa1f093e6 ("ASoC: SOF:
> Add a new IPC op for parsing topology manifest"). If I am tracking all
> my patches correctly, this is the only instance of -Wformat that does
> not have a patch applied to a maintainer's tree so it would be really
> unfortunate if we could not sure it on for -rc1.

> We could probably route this via the Kbuild tree with an Ack along with
> the patch that enables -Wformat if it cannot go via Mark's or Takashi's
> ree.

We have a couple of months to get fixes into the next release so it's
not an emergency at this point.  If you want people to see things
promptly you really need to do things like send them with subject lines
that look like something that might be relevant for them to review
(generally, at least visually resemble how other commits in the area
look).  I don't know if I even opened this mail first time around
because based on the subject it looks like something for drivers/soc not
a subsystem I maintain.  Given that none of the SOF people responded
it's likely something similar applied there.

In any case, if you think something's been lost the content free pings
either here or on IRC aren't that helpful - they're not directly
actionable.  It is generally more helpful to resend, that way people
have the patch to hand and don't need to go looking for it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

end of thread, other threads:[~2022-08-03 11:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-21 21:12 [PATCH] soc: sof: fix clang -Wformat warnings Justin Stitt
2022-07-21 21:22 ` Nathan Chancellor
2022-08-02 20:25 ` Nathan Chancellor
2022-08-03 11:44   ` Mark Brown

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).