[bug report] ASoC: SOF: ipc4-control: Add support for bytes control get and put
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
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
participants (2)
-
Dan Carpenter
-
Péter Ujfalusi