drivers/staging/greybus/audio_topology.c | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)
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.0This 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
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
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
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
© 2016 - 2026 Red Hat, Inc.