[v1] staging: greybus: audio: avoid snprintf truncation warnings

[PATCH] staging: greybus: audio: avoid snprintf truncation warnings

Posted by Sun Jian 1 month, 1 week ago

W=1 reports possible truncation when formatting widget and control names
using snprintf() with a %s argument and fixed-size buffers. Build the
prefixed names using scnprintf() plus strlcat() instead, so truncation,
if any, is handled by the string helpers rather than during printf
formatting.

No functional change intended.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 drivers/staging/greybus/audio_topology.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c
index 76146f91cddc..4293ab899390 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -1087,7 +1087,8 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
 
 	/* Prefix dev_id to widget control_name */
 	strscpy(temp_name, w->name, sizeof(temp_name));
-	snprintf(w->name, sizeof(w->name), "GB %d %s", module->dev_id, temp_name);
+	scnprintf(w->name, sizeof(w->name), "GB %d ", module->dev_id);
+	strlcat(w->name, temp_name, sizeof(w->name));
 
 	switch (w->type) {
 	case snd_soc_dapm_spk:
@@ -1169,8 +1170,8 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module,
 		control->id = curr->id;
 		/* Prefix dev_id to widget_name */
 		strscpy(temp_name, curr->name, sizeof(temp_name));
-		snprintf(curr->name, sizeof(curr->name), "GB %d %s", module->dev_id,
-			 temp_name);
+		scnprintf(curr->name, sizeof(curr->name), "GB %d ", module->dev_id);
+		strlcat(curr->name, temp_name, sizeof(curr->name));
 		control->name = curr->name;
 		if (curr->info.type == GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED) {
 			struct gb_audio_enumerated *gbenum =
-- 
2.43.0

Re: [PATCH] staging: greybus: audio: avoid snprintf truncation warnings

Posted by David Laight 1 month, 1 week ago

On Mon, 29 Dec 2025 19:26:49 +0800
Sun Jian <sun.jian.kdev@gmail.com> wrote:

> W=1 reports possible truncation when formatting widget and control names
> using snprintf() with a %s argument and fixed-size buffers. Build the
> prefixed names using scnprintf() plus strlcat() instead, so truncation,
> if any, is handled by the string helpers rather than during printf
> formatting.

That is just brain dead - more so than the fact the the warning is pretty
hard to ignore.
Commonning up the code as:
static void prefix_div_id(char *name, size_t name_len, unsigned int dev_id)
{
	char temp_name[32], *cp = temp_name;

	strscpy(temp_name, name, sizeof temp_name);
	OPTIMISER_HIDE_VAR(cp);
	snprintf(name, name_len, "GB %d %s", dev_id, cp);
}
should be enough and is about the best you can do.
Possibly worth a comment that name_len is expected to be 32.

The way this code processes the tplg_data isn't nice at all.
Convert the offsets to correctly typed pointers as soon as possible.
You might want to verify that the separate arrays don't overlap.

There is also the:
	curr = (void *)curr + w_size;
which AFIACT is just 'curr++';

	David

> 
> No functional change intended.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  drivers/staging/greybus/audio_topology.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c
> index 76146f91cddc..4293ab899390 100644
> --- a/drivers/staging/greybus/audio_topology.c
> +++ b/drivers/staging/greybus/audio_topology.c
> @@ -1087,7 +1087,8 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
>  
>  	/* Prefix dev_id to widget control_name */
>  	strscpy(temp_name, w->name, sizeof(temp_name));
> -	snprintf(w->name, sizeof(w->name), "GB %d %s", module->dev_id, temp_name);
> +	scnprintf(w->name, sizeof(w->name), "GB %d ", module->dev_id);
> +	strlcat(w->name, temp_name, sizeof(w->name));
>  
>  	switch (w->type) {
>  	case snd_soc_dapm_spk:
> @@ -1169,8 +1170,8 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module,
>  		control->id = curr->id;
>  		/* Prefix dev_id to widget_name */
>  		strscpy(temp_name, curr->name, sizeof(temp_name));
> -		snprintf(curr->name, sizeof(curr->name), "GB %d %s", module->dev_id,
> -			 temp_name);
> +		scnprintf(curr->name, sizeof(curr->name), "GB %d ", module->dev_id);
> +		strlcat(curr->name, temp_name, sizeof(curr->name));
>  		control->name = curr->name;
>  		if (curr->info.type == GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED) {
>  			struct gb_audio_enumerated *gbenum =

[PATCH v3] staging: greybus: audio: avoid snprintf truncation warnings

Posted by Sun Jian 1 month, 1 week ago

W=1 reports possible truncation when formatting widget/control names
with snprintf() and a %s argument. Use a small helper and hide the %s
pointer from the compiler's truncation analysis via OPTIMIZER_HIDE_VAR(),
while keeping the existing snprintf() formatting.

No functional change intended.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>

Changes in v3:
- Replace the earlier scnprintf()/strlcat() approach with a helper
  keeping snprintf().
- Hide the %s argument from compiler truncation analysis using
  OPTIMIZER_HIDE_VAR().
- Add a small local length limit macro with a short comment.
---
 drivers/staging/greybus/audio_topology.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c
index 76146f91cddc..35775067897c 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -1009,6 +1009,19 @@ static const struct snd_soc_dapm_widget gbaudio_widgets[] = {
 					SND_SOC_DAPM_POST_PMD),
 };
 
+/* Limit %s length to avoid -Wformat-truncation with snprintf() */
+#define GB_NAME_TMP_LEN 32
+
+static void gbaudio_prefix_dev_id(char *name, size_t name_len,
+				  unsigned int dev_id)
+{
+	char temp_name[GB_NAME_TMP_LEN], *cp = temp_name;
+
+	strscpy(temp_name, name, sizeof(temp_name));
+	OPTIMIZER_HIDE_VAR(cp);
+	snprintf(name, name_len, "GB %u %s", dev_id, cp);
+}
+
 static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
 				      struct snd_soc_dapm_widget *dw,
 				      struct gb_audio_widget *w, int *w_size)
@@ -1018,7 +1031,6 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
 	struct gb_audio_control *curr;
 	struct gbaudio_control *control, *_control;
 	size_t size;
-	char temp_name[NAME_SIZE];
 
 	ret = gbaudio_validate_kcontrol_count(w);
 	if (ret) {
@@ -1086,8 +1098,7 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
 	}
 
 	/* Prefix dev_id to widget control_name */
-	strscpy(temp_name, w->name, sizeof(temp_name));
-	snprintf(w->name, sizeof(w->name), "GB %d %s", module->dev_id, temp_name);
+	gbaudio_prefix_dev_id(w->name, sizeof(w->name), module->dev_id);
 
 	switch (w->type) {
 	case snd_soc_dapm_spk:
@@ -1143,7 +1154,6 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module,
 	struct gb_audio_control *curr;
 	struct gbaudio_control *control, *_control;
 	size_t size;
-	char temp_name[NAME_SIZE];
 
 	size = sizeof(struct snd_kcontrol_new) * module->num_controls;
 	dapm_kctls = devm_kzalloc(module->dev, size, GFP_KERNEL);
@@ -1168,9 +1178,7 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module,
 		}
 		control->id = curr->id;
 		/* Prefix dev_id to widget_name */
-		strscpy(temp_name, curr->name, sizeof(temp_name));
-		snprintf(curr->name, sizeof(curr->name), "GB %d %s", module->dev_id,
-			 temp_name);
+		gbaudio_prefix_dev_id(curr->name, sizeof(curr->name), module->dev_id);
 		control->name = curr->name;
 		if (curr->info.type == GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED) {
 			struct gb_audio_enumerated *gbenum =
-- 
2.43.0

Re: [PATCH v3] staging: greybus: audio: avoid snprintf truncation warnings

Posted by Dan Carpenter 1 month ago

This W=1 string truncation warnings always seems like a pointless thing.
dmesg output is really only intended for developers.  I don't even know
how to look at the dmesg on my phone or kindle.  Who cares if the last
character in a really long device name is missing?  I have looked at a
lot of stack traces and I have never once been stymied because one
character was missing in a really long device name.

And also these are 90% false positives.  We just add bounds checking all
of our output to prevent memory corruption and not because we think that
we'll actually hit the boundaries.  And the GCC is not able to analyze
these in a sensible way, it's just going based on the variable types.

This patch makes the code worse (more complicated).

To recap:
1: It's warning about a non-issue.  No one cares about truncated output.
2: The warning is implemented poorly.  High false positive ratio.
3: The fix makes the code worse

Just fix the tool instead of making the code worse for no reason.

regards,
dan carpenter

Re: [PATCH v3] staging: greybus: audio: avoid snprintf truncation warnings

Posted by sun jian 1 month ago

Hi Dan,

Understood — I'll drop this patch. Thanks.

regards,
Sun Jian

On Mon, Jan 5, 2026 at 7:55 PM Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> This W=1 string truncation warnings always seems like a pointless thing.
> dmesg output is really only intended for developers.  I don't even know
> how to look at the dmesg on my phone or kindle.  Who cares if the last
> character in a really long device name is missing?  I have looked at a
> lot of stack traces and I have never once been stymied because one
> character was missing in a really long device name.
>
> And also these are 90% false positives.  We just add bounds checking all
> of our output to prevent memory corruption and not because we think that
> we'll actually hit the boundaries.  And the GCC is not able to analyze
> these in a sensible way, it's just going based on the variable types.
>
> This patch makes the code worse (more complicated).
>
> To recap:
> 1: It's warning about a non-issue.  No one cares about truncated output.
> 2: The warning is implemented poorly.  High false positive ratio.
> 3: The fix makes the code worse
>
> Just fix the tool instead of making the code worse for no reason.
>
> regards,
> dan carpenter

Re: [PATCH v3] staging: greybus: audio: avoid snprintf truncation warnings

Posted by Greg Kroah-Hartman 1 month, 1 week ago

On Tue, Dec 30, 2025 at 09:29:08AM +0800, Sun Jian wrote:
> W=1 reports possible truncation when formatting widget/control names
> with snprintf() and a %s argument. Use a small helper and hide the %s
> pointer from the compiler's truncation analysis via OPTIMIZER_HIDE_VAR(),
> while keeping the existing snprintf() formatting.
> 
> No functional change intended.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> 
> Changes in v3:
> - Replace the earlier scnprintf()/strlcat() approach with a helper
>   keeping snprintf().
> - Hide the %s argument from compiler truncation analysis using
>   OPTIMIZER_HIDE_VAR().
> - Add a small local length limit macro with a short comment.
> ---

The "changes" go below the --- line, as the documentation asks for.  And
please include what changed from versions prior to that as well.

But:

>  drivers/staging/greybus/audio_topology.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c
> index 76146f91cddc..35775067897c 100644
> --- a/drivers/staging/greybus/audio_topology.c
> +++ b/drivers/staging/greybus/audio_topology.c
> @@ -1009,6 +1009,19 @@ static const struct snd_soc_dapm_widget gbaudio_widgets[] = {
>  					SND_SOC_DAPM_POST_PMD),
>  };
>  
> +/* Limit %s length to avoid -Wformat-truncation with snprintf() */
> +#define GB_NAME_TMP_LEN 32
> +
> +static void gbaudio_prefix_dev_id(char *name, size_t name_len,
> +				  unsigned int dev_id)
> +{
> +	char temp_name[GB_NAME_TMP_LEN], *cp = temp_name;
> +
> +	strscpy(temp_name, name, sizeof(temp_name));
> +	OPTIMIZER_HIDE_VAR(cp);

What?  Why?  That feels wrong.  Let's not add hacks for broken
compilers.

> +	snprintf(name, name_len, "GB %u %s", dev_id, cp);
> +}
> +
>  static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
>  				      struct snd_soc_dapm_widget *dw,
>  				      struct gb_audio_widget *w, int *w_size)
> @@ -1018,7 +1031,6 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
>  	struct gb_audio_control *curr;
>  	struct gbaudio_control *control, *_control;
>  	size_t size;
> -	char temp_name[NAME_SIZE];
>  
>  	ret = gbaudio_validate_kcontrol_count(w);
>  	if (ret) {
> @@ -1086,8 +1098,7 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
>  	}
>  
>  	/* Prefix dev_id to widget control_name */
> -	strscpy(temp_name, w->name, sizeof(temp_name));
> -	snprintf(w->name, sizeof(w->name), "GB %d %s", module->dev_id, temp_name);
> +	gbaudio_prefix_dev_id(w->name, sizeof(w->name), module->dev_id);

This feels like a broken tool, let's not do foolish things just to make
compilers quiet.  W=1 is not a good reason to just make things "silent"
by moving code around like you did here.

sorry,

greg k-h

Re: [PATCH v3] staging: greybus: audio: avoid snprintf truncation warnings

Posted by David Laight 1 month, 1 week ago

On Tue, 30 Dec 2025 08:40:48 +0100
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Tue, Dec 30, 2025 at 09:29:08AM +0800, Sun Jian wrote:
> > W=1 reports possible truncation when formatting widget/control names
> > with snprintf() and a %s argument. Use a small helper and hide the %s
> > pointer from the compiler's truncation analysis via OPTIMIZER_HIDE_VAR(),
> > while keeping the existing snprintf() formatting.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> > 
> > Changes in v3:
> > - Replace the earlier scnprintf()/strlcat() approach with a helper
> >   keeping snprintf().
> > - Hide the %s argument from compiler truncation analysis using
> >   OPTIMIZER_HIDE_VAR().
> > - Add a small local length limit macro with a short comment.
> > ---  
> 
> The "changes" go below the --- line, as the documentation asks for.  And
> please include what changed from versions prior to that as well.
> 
> But:
> 
> >  drivers/staging/greybus/audio_topology.c | 22 +++++++++++++++-------
> >  1 file changed, 15 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c
> > index 76146f91cddc..35775067897c 100644
> > --- a/drivers/staging/greybus/audio_topology.c
> > +++ b/drivers/staging/greybus/audio_topology.c
> > @@ -1009,6 +1009,19 @@ static const struct snd_soc_dapm_widget gbaudio_widgets[] = {
> >  					SND_SOC_DAPM_POST_PMD),
> >  };
> >  
> > +/* Limit %s length to avoid -Wformat-truncation with snprintf() */
> > +#define GB_NAME_TMP_LEN 32
> > +
> > +static void gbaudio_prefix_dev_id(char *name, size_t name_len,
> > +				  unsigned int dev_id)
> > +{
> > +	char temp_name[GB_NAME_TMP_LEN], *cp = temp_name;
> > +
> > +	strscpy(temp_name, name, sizeof(temp_name));
> > +	OPTIMIZER_HIDE_VAR(cp);  
> 
> What?  Why?  That feels wrong.  Let's not add hacks for broken
> compilers.

I don't like it either.
But I don't know of any other way of silencing that compiler warning.
It is actually a real PITA and mostly pointless.
After all it only warns about strings it knows the maximum size of.
It is more likely that a truncation 'bug' will happen for an indefinite
length string.
After all, the whole point about snprintf() is that it truncates.
I can well imagine that trying to stop the 'format overflow' warning
has actually created bugs!

	David


> 
> > +	snprintf(name, name_len, "GB %u %s", dev_id, cp);
> > +}
> > +
> >  static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
> >  				      struct snd_soc_dapm_widget *dw,
> >  				      struct gb_audio_widget *w, int *w_size)
> > @@ -1018,7 +1031,6 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
> >  	struct gb_audio_control *curr;
> >  	struct gbaudio_control *control, *_control;
> >  	size_t size;
> > -	char temp_name[NAME_SIZE];
> >  
> >  	ret = gbaudio_validate_kcontrol_count(w);
> >  	if (ret) {
> > @@ -1086,8 +1098,7 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
> >  	}
> >  
> >  	/* Prefix dev_id to widget control_name */
> > -	strscpy(temp_name, w->name, sizeof(temp_name));
> > -	snprintf(w->name, sizeof(w->name), "GB %d %s", module->dev_id, temp_name);
> > +	gbaudio_prefix_dev_id(w->name, sizeof(w->name), module->dev_id);  
> 
> This feels like a broken tool, let's not do foolish things just to make
> compilers quiet.  W=1 is not a good reason to just make things "silent"
> by moving code around like you did here.
> 
> sorry,
> 
> greg k-h

[PATCH v2] staging: greybus: audio: avoid snprintf truncation warnings

Posted by Sun Jian 1 month, 1 week ago

W=1 reports possible truncation when formatting widget/control names
with snprintf() and a %s argument. Use a small helper and hide the %s
pointer from the compiler's truncation analysis via OPTIMIZER_HIDE_VAR(),
while keeping the existing snprintf() formatting.

No functional change intended.

Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
---
 drivers/staging/greybus/audio_topology.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c
index 76146f91cddc..4293ab899390 100644
--- a/drivers/staging/greybus/audio_topology.c
+++ b/drivers/staging/greybus/audio_topology.c
@@ -1087,7 +1087,8 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
 
 	/* Prefix dev_id to widget control_name */
 	strscpy(temp_name, w->name, sizeof(temp_name));
-	snprintf(w->name, sizeof(w->name), "GB %d %s", module->dev_id, temp_name);
+	scnprintf(w->name, sizeof(w->name), "GB %d ", module->dev_id);
+	strlcat(w->name, temp_name, sizeof(w->name));
 
 	switch (w->type) {
 	case snd_soc_dapm_spk:
@@ -1169,8 +1170,8 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module,
 		control->id = curr->id;
 		/* Prefix dev_id to widget_name */
 		strscpy(temp_name, curr->name, sizeof(temp_name));
-		snprintf(curr->name, sizeof(curr->name), "GB %d %s", module->dev_id,
-			 temp_name);
+		scnprintf(curr->name, sizeof(curr->name), "GB %d ", module->dev_id);
+		strlcat(curr->name, temp_name, sizeof(curr->name));
 		control->name = curr->name;
 		if (curr->info.type == GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED) {
 			struct gb_audio_enumerated *gbenum =
-- 
2.43.0

Re: [PATCH v2] staging: greybus: audio: avoid snprintf truncation warnings

Posted by Greg Kroah-Hartman 1 month, 1 week ago

On Tue, Dec 30, 2025 at 12:13:46AM +0800, Sun Jian wrote:
> W=1 reports possible truncation when formatting widget/control names
> with snprintf() and a %s argument. Use a small helper and hide the %s
> pointer from the compiler's truncation analysis via OPTIMIZER_HIDE_VAR(),
> while keeping the existing snprintf() formatting.
> 
> No functional change intended.
> 
> Signed-off-by: Sun Jian <sun.jian.kdev@gmail.com>
> ---
>  drivers/staging/greybus/audio_topology.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/audio_topology.c b/drivers/staging/greybus/audio_topology.c
> index 76146f91cddc..4293ab899390 100644
> --- a/drivers/staging/greybus/audio_topology.c
> +++ b/drivers/staging/greybus/audio_topology.c
> @@ -1087,7 +1087,8 @@ static int gbaudio_tplg_create_widget(struct gbaudio_module_info *module,
>  
>  	/* Prefix dev_id to widget control_name */
>  	strscpy(temp_name, w->name, sizeof(temp_name));
> -	snprintf(w->name, sizeof(w->name), "GB %d %s", module->dev_id, temp_name);
> +	scnprintf(w->name, sizeof(w->name), "GB %d ", module->dev_id);
> +	strlcat(w->name, temp_name, sizeof(w->name));
>  
>  	switch (w->type) {
>  	case snd_soc_dapm_spk:
> @@ -1169,8 +1170,8 @@ static int gbaudio_tplg_process_kcontrols(struct gbaudio_module_info *module,
>  		control->id = curr->id;
>  		/* Prefix dev_id to widget_name */
>  		strscpy(temp_name, curr->name, sizeof(temp_name));
> -		snprintf(curr->name, sizeof(curr->name), "GB %d %s", module->dev_id,
> -			 temp_name);
> +		scnprintf(curr->name, sizeof(curr->name), "GB %d ", module->dev_id);
> +		strlcat(curr->name, temp_name, sizeof(curr->name));
>  		control->name = curr->name;
>  		if (curr->info.type == GB_AUDIO_CTL_ELEM_TYPE_ENUMERATED) {
>  			struct gb_audio_enumerated *gbenum =
> -- 
> 2.43.0
> 
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot