[PATCH] ASoC: mediatek: mt2701: fix snprintf bounds

[PATCH] ASoC: mediatek: mt2701: fix snprintf bounds

[Rosen Penev]

For whatever reason, GCC is unable to figure out that i2s_num is a
single digit number, with MT2701_BASE_CLK_NUM being the maximum value it
represents. Add a min() call to help it out and fix W=1 errors regarding
snprintf bounds.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c b/sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c
index 5a2bcf027b4f..43157f218409 100644
--- a/sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c
+++ b/sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c
@@ -25,6 +25,7 @@ static const char *const base_clks[] = {
 int mt2701_init_clock(struct mtk_base_afe *afe)
 {
 	struct mt2701_afe_private *afe_priv = afe->platform_priv;
+	int i2s_num;
 	int i;
 
 	for (i = 0; i < MT2701_BASE_CLK_NUM; i++) {
@@ -35,8 +36,9 @@ int mt2701_init_clock(struct mtk_base_afe *afe)
 		}
 	}
 
+	i2s_num = min(MT2701_BASE_CLK_NUM, afe_priv->soc->i2s_num);
 	/* Get I2S related clocks */
-	for (i = 0; i < afe_priv->soc->i2s_num; i++) {
+	for (i = 0; i < i2s_num; i++) {
 		struct mt2701_i2s_path *i2s_path = &afe_priv->i2s_path[i];
 		struct clk *i2s_ck;
 		char name[13];
-- 
2.54.0

Re: [PATCH] ASoC: mediatek: mt2701: fix snprintf bounds

[Mark Brown]

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

On Mon, May 18, 2026 at 06:04:40PM -0700, Rosen Penev wrote:
> For whatever reason, GCC is unable to figure out that i2s_num is a
> single digit number, with MT2701_BASE_CLK_NUM being the maximum value it
> represents. Add a min() call to help it out and fix W=1 errors regarding
> snprintf bounds.

> +	i2s_num = min(MT2701_BASE_CLK_NUM, afe_priv->soc->i2s_num);
>  	/* Get I2S related clocks */
> -	for (i = 0; i < afe_priv->soc->i2s_num; i++) {
> +	for (i = 0; i < i2s_num; i++) {

This makes the code rather obscure and will start silently failing if we
ever get more than 9 clocks, we should at least put a build time assert
or a warn on in there.

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

Re: [PATCH] ASoC: mediatek: mt2701: fix snprintf bounds

[Rosen Penev]

On Tue, May 19, 2026 at 1:33 AM Mark Brown <broonie@kernel.org> wrote:
>
> On Mon, May 18, 2026 at 06:04:40PM -0700, Rosen Penev wrote:
> > For whatever reason, GCC is unable to figure out that i2s_num is a
> > single digit number, with MT2701_BASE_CLK_NUM being the maximum value it
> > represents. Add a min() call to help it out and fix W=1 errors regarding
> > snprintf bounds.
>
> > +     i2s_num = min(MT2701_BASE_CLK_NUM, afe_priv->soc->i2s_num);
> >       /* Get I2S related clocks */
> > -     for (i = 0; i < afe_priv->soc->i2s_num; i++) {
> > +     for (i = 0; i < i2s_num; i++) {
>
> This makes the code rather obscure and will start silently failing if we
> ever get more than 9 clocks, we should at least put a build time assert
> or a warn on in there.
how so? MT2701_BASE_CLK_NUM would increase.

Re: [PATCH] ASoC: mediatek: mt2701: fix snprintf bounds

[David Laight]

On Mon, 18 May 2026 18:04:40 -0700
Rosen Penev <rosenp@gmail.com> wrote:

> For whatever reason, GCC is unable to figure out that i2s_num is a
> single digit number, with MT2701_BASE_CLK_NUM being the maximum value it
> represents. Add a min() call to help it out and fix W=1 errors regarding
> snprintf bounds.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c b/sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c
> index 5a2bcf027b4f..43157f218409 100644
> --- a/sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c
> +++ b/sound/soc/mediatek/mt2701/mt2701-afe-clock-ctrl.c
> @@ -25,6 +25,7 @@ static const char *const base_clks[] = {
>  int mt2701_init_clock(struct mtk_base_afe *afe)
>  {
>  	struct mt2701_afe_private *afe_priv = afe->platform_priv;
> +	int i2s_num;
>  	int i;
>  
>  	for (i = 0; i < MT2701_BASE_CLK_NUM; i++) {
> @@ -35,8 +36,9 @@ int mt2701_init_clock(struct mtk_base_afe *afe)
>  		}
>  	}
>  
> +	i2s_num = min(MT2701_BASE_CLK_NUM, afe_priv->soc->i2s_num);

To me that is backwards, like an 'if' put the variable before the limit.

>  	/* Get I2S related clocks */
> -	for (i = 0; i < afe_priv->soc->i2s_num; i++) {
> +	for (i = 0; i < i2s_num; i++) {

Caching the limit also stops the compiler having to read it every iteration.

-- David

>  		struct mt2701_i2s_path *i2s_path = &afe_priv->i2s_path[i];
>  		struct clk *i2s_ck;
>  		char name[13];