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

[David Laight]

On Tue, 30 Dec 2025 08:40:48 +0100
Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> > 
> > 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