[bug report] ASoC: SOF: ipc4-control: Add support for bytes control get and put

[bug report] ASoC: SOF: ipc4-control: Add support for bytes control get and put

[Dan Carpenter]

Hello Peter Ujfalusi,

The patch a062c8899fed: "ASoC: SOF: ipc4-control: Add support for
bytes control get and put" from Mar 13, 2023, leads to the following
Smatch static checker warning:

	sound/soc/sof/ipc4-control.c:436 sof_ipc4_widget_kcontrol_setup()
	warn: iterator used outside loop: 'scontrol'

sound/soc/sof/ipc4-control.c
    411 static int sof_ipc4_widget_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
    412 {
    413         struct snd_sof_control *scontrol;
    414         int ret = 0;
    415 
    416         list_for_each_entry(scontrol, &sdev->kcontrol_list, list) {
    417                 if (scontrol->comp_id == swidget->comp_id) {
    418                         switch (scontrol->info_type) {
    419                         case SND_SOC_TPLG_CTL_VOLSW:
    420                         case SND_SOC_TPLG_CTL_VOLSW_SX:
    421                         case SND_SOC_TPLG_CTL_VOLSW_XR_SX:
    422                                 ret = sof_ipc4_set_volume_data(sdev, swidget,
    423                                                                scontrol, false);
    424                                 break;
    425                         case SND_SOC_TPLG_CTL_BYTES:
    426                                 ret = sof_ipc4_set_get_bytes_data(sdev, scontrol,
    427                                                                   true, false);
    428                                 break;

This breaks out of the switch statement and not the loop.  So that means
that it will continue through the loop and ret is reset.

    429                         default:
    430                                 break;
    431                         }
    432                 }
    433         }
    434 
    435         if (ret < 0)
--> 436                 dev_err(sdev->dev, "kcontrol %d set up failed for widget %s\n",
    437                         scontrol->comp_id, swidget->widget->name);
                                ^^^^^^^^^^^^^^^^^
"scontrol" cannot be a valid pointer at this stage.  It is always an
offset off the &sdev->kcontrol_list because of the above issue.

    438 
    439         return ret;
    440 }

regards,
dan carpenter

Re: [bug report] ASoC: SOF: ipc4-control: Add support for bytes control get and put

[Péter Ujfalusi]

Hi Dan,

On 21/03/2023 09:21, Dan Carpenter wrote:
> Hello Peter Ujfalusi,
> 
> The patch a062c8899fed: "ASoC: SOF: ipc4-control: Add support for
> bytes control get and put" from Mar 13, 2023, leads to the following
> Smatch static checker warning:
> 
> 	sound/soc/sof/ipc4-control.c:436 sof_ipc4_widget_kcontrol_setup()
> 	warn: iterator used outside loop: 'scontrol'
> 
> sound/soc/sof/ipc4-control.c
>     411 static int sof_ipc4_widget_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
>     412 {
>     413         struct snd_sof_control *scontrol;
>     414         int ret = 0;
>     415 
>     416         list_for_each_entry(scontrol, &sdev->kcontrol_list, list) {
>     417                 if (scontrol->comp_id == swidget->comp_id) {
>     418                         switch (scontrol->info_type) {
>     419                         case SND_SOC_TPLG_CTL_VOLSW:
>     420                         case SND_SOC_TPLG_CTL_VOLSW_SX:
>     421                         case SND_SOC_TPLG_CTL_VOLSW_XR_SX:
>     422                                 ret = sof_ipc4_set_volume_data(sdev, swidget,
>     423                                                                scontrol, false);
>     424                                 break;
>     425                         case SND_SOC_TPLG_CTL_BYTES:
>     426                                 ret = sof_ipc4_set_get_bytes_data(sdev, scontrol,
>     427                                                                   true, false);
>     428                                 break;
> 
> This breaks out of the switch statement and not the loop.  So that means
> that it will continue through the loop and ret is reset.
> 
>     429                         default:
>     430                                 break;
>     431                         }
>     432                 }
>     433         }
>     434 
>     435         if (ret < 0)
> --> 436                 dev_err(sdev->dev, "kcontrol %d set up failed for widget %s\n",
>     437                         scontrol->comp_id, swidget->widget->name);
>                                 ^^^^^^^^^^^^^^^^^
> "scontrol" cannot be a valid pointer at this stage.  It is always an
> offset off the &sdev->kcontrol_list because of the above issue.

This is valid in a semantics sense and I will fix it.
In real life we have single control per swidget and this should have
been one step up.

> 
>     438 
>     439         return ret;
>     440 }
> 
> regards,
> dan carpenter

-- 
Péter