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

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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