| Message ID | 20240614153717.30143-5-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ALSA: some driver fixes for control input validations | expand |
On 14. 06. 24 17:37, Takashi Iwai wrote: > The control elements with volatile flag don't guarantee that the > written values are actually saved for the next reads, hence we can't > verify the written values reliably. Skip the verification after write > tests for those volatile controls for avoiding confusion. > > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > Closes: https://lore.kernel.org/r/1d44be36-9bb9-4d82-8953-5ae2a4f09405@molgen.mpg.de > Signed-off-by: Takashi Iwai <tiwai@suse.de> Reviewed-by: Jaroslav Kysela <perex@perex.cz>
On Fri, Jun 14, 2024 at 05:37:13PM +0200, Takashi Iwai wrote: > @@ -616,6 +616,10 @@ static int write_and_verify(struct ctl_data *ctl, > if (!snd_ctl_elem_info_is_readable(ctl->info)) > return err; > > + /* Skip the verification for volatile controls, too */ > + if (snd_ctl_elem_info_is_volatile(ctl->info)) > + return err; > + I think we should do the checks in test_ctl_get_value() still - a read and then ctl_value_is_valid() on whatever we read. It doesn't need to match the value we wrote but it should still be valid for the control.
On Fri, 14 Jun 2024 17:57:37 +0200, Mark Brown wrote: > > On Fri, Jun 14, 2024 at 05:37:13PM +0200, Takashi Iwai wrote: > > > @@ -616,6 +616,10 @@ static int write_and_verify(struct ctl_data *ctl, > > if (!snd_ctl_elem_info_is_readable(ctl->info)) > > return err; > > > > + /* Skip the verification for volatile controls, too */ > > + if (snd_ctl_elem_info_is_volatile(ctl->info)) > > + return err; > > + > > I think we should do the checks in test_ctl_get_value() still - a read > and then ctl_value_is_valid() on whatever we read. It doesn't need to > match the value we wrote but it should still be valid for the control. So something like below? Takashi -- 8< -- From: Takashi Iwai <tiwai@suse.de> Subject: [PATCH v3] kselftest/alsa: mixer-test: Allow value mismatch for volatile controls The control elements with volatile flag don't guarantee that the written values are actually saved for the next reads, hence we can't verify the written values reliably. Return as success for volatile controls even if the value verification after writes fails, in order to avoid false-positive. Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> Closes: https://lore.kernel.org/r/1d44be36-9bb9-4d82-8953-5ae2a4f09405@molgen.mpg.de Signed-off-by: Takashi Iwai <tiwai@suse.de> --- tools/testing/selftests/alsa/mixer-test.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c index 1c04e5f638a0..62b77737f0de 100644 --- a/tools/testing/selftests/alsa/mixer-test.c +++ b/tools/testing/selftests/alsa/mixer-test.c @@ -668,6 +668,10 @@ static int write_and_verify(struct ctl_data *ctl, ksft_print_msg("%s read and written values differ\n", ctl->name); + /* Allow difference for volatile controls */ + if (snd_ctl_elem_info_is_volatile(ctl->info)) + return 0; + return -1; }
On Fri, Jun 14, 2024 at 06:08:12PM +0200, Takashi Iwai wrote: > Mark Brown wrote: > +++ b/tools/testing/selftests/alsa/mixer-test.c > @@ -668,6 +668,10 @@ static int write_and_verify(struct ctl_data *ctl, > ksft_print_msg("%s read and written values differ\n", > ctl->name); > > + /* Allow difference for volatile controls */ > + if (snd_ctl_elem_info_is_volatile(ctl->info)) > + return 0; > + > return -1; > } That'll still print the warnings about the values differing and won't check the values are in range... I'll send a patch.
diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c index 1c04e5f638a0..c98167818319 100644 --- a/tools/testing/selftests/alsa/mixer-test.c +++ b/tools/testing/selftests/alsa/mixer-test.c @@ -616,6 +616,10 @@ static int write_and_verify(struct ctl_data *ctl, if (!snd_ctl_elem_info_is_readable(ctl->info)) return err; + /* Skip the verification for volatile controls, too */ + if (snd_ctl_elem_info_is_volatile(ctl->info)) + return err; + snd_ctl_elem_value_set_id(read_val, ctl->id); err = snd_ctl_elem_read(ctl->card->handle, read_val);
The control elements with volatile flag don't guarantee that the written values are actually saved for the next reads, hence we can't verify the written values reliably. Skip the verification after write tests for those volatile controls for avoiding confusion. Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> Closes: https://lore.kernel.org/r/1d44be36-9bb9-4d82-8953-5ae2a4f09405@molgen.mpg.de Signed-off-by: Takashi Iwai <tiwai@suse.de> --- tools/testing/selftests/alsa/mixer-test.c | 4 ++++ 1 file changed, 4 insertions(+)