| Message ID | 20211208095209.1772296-1-perex@perex.cz (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kselftest: alsa: Use private alsa-lib configuration in mixer test | expand |
On Wed, Dec 08, 2021 at 10:52:09AM +0100, Jaroslav Kysela wrote: > As mentined by Takashi Sakamoto, the system-wide alsa-lib configuration > may override the standard device declarations. This patch use the private > alsa-lib configuration to set the predictable environment. Reviewed-by: Mark Brown <broonie@kernel.org>
On Wed, Dec 8, 2021, at 18:52, Jaroslav Kysela wrote: > As mentined by Takashi Sakamoto, the system-wide alsa-lib configuration > may override the standard device declarations. This patch use the private > alsa-lib configuration to set the predictable environment. > > Cc: Mark Brown <broonie@kernel.org> > Cc: Shuah Khan <shuah@kernel.org> > Cc: Takashi Sakamoto <o-takashi@sakamocchi.jp> > Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > Cc: linux-kselftest@vger.kernel.org > Link: https://lore.kernel.org/alsa-devel/Ya7TAHdMe9i41bsC@workstation/ > Signed-off-by: Jaroslav Kysela <perex@perex.cz> > --- > tools/testing/selftests/alsa/mixer-test.c | 50 ++++++++++++++++++++++- > 1 file changed, 49 insertions(+), 1 deletion(-) I'm not positively for the patch since it can take developers puzzled due to the symbol dependency newly introduced in unreleased version of alsa-lib. It's better to check the version of alsa-lib in Makefile to avoid developers’dole if we have enough respect to embedded developers, especially forced to work with legacy userspace. (and it often occurs). As a side note, I think it better to change symbol table (alsa-lib/src/Versions.in) if introducing new public symbol of c library. Regards Takashi Sakamoto
On Wed, Dec 08, 2021 at 10:55:41PM +0900, Takashi Sakamoto wrote: > I'm not positively for the patch since it can take developers puzzled due to > the symbol dependency newly introduced in unreleased version of alsa-lib. Shouldn't the version check and local definition avoid that issue - if the version of alsa-lib doesn't have snd_config_load_string() then we'll use a locally defined version of snd_config_load_string() and not depend on the alsa-lib symbol? > It's better to check the version of alsa-lib in Makefile to avoid developers’dole > if we have enough respect to embedded developers, especially forced to work > with legacy userspace. (and it often occurs). Or just avoid using fancy new library features - if we need to limit the test to only building with bleeding edge versions that gets restrictive. TBH this is probably even more painful for people working with enterprise distros than embedded systems, if you're building everything for your target it's not usually too bad to drop in an updated version of something like alsa-lib but if you're using disro binaries it's less idiomatic. Either way it's a barrier though.
On 08. 12. 21 15:14, Mark Brown wrote: > On Wed, Dec 08, 2021 at 10:55:41PM +0900, Takashi Sakamoto wrote: > >> I'm not positively for the patch since it can take developers puzzled due to >> the symbol dependency newly introduced in unreleased version of alsa-lib. > > Shouldn't the version check and local definition avoid that issue - if > the version of alsa-lib doesn't have snd_config_load_string() then we'll > use a locally defined version of snd_config_load_string() and not depend > on the alsa-lib symbol? The 1.2.6 library is released. The goal was to allow to run code with the older libraries until the new version is more spread. It's just one #if in the source code and a 1:1 code copy. The kselftest packagers should define the proper package dependencies for the downgrade possibility - it's distribution specific thing. Anyway, the dynamic linker will print the correct error when the user tries to run the test program compiled using the new library on the system with the older library. Jaroslav
On Wed, Dec 08, 2021 at 10:52:09AM +0100, Jaroslav Kysela wrote:
> +#if !defined(SND_LIB_VER) || SND_LIB_VERSION < SND_LIB_VER(1, 2, 6)
This barfs if the local definition is used since the preprocessor will
try to evaluate SND_LIB_VER even if the macro is not defined and the
left hand side of the || is true:
mixer-test.c:66:60: error: missing binary operator before token "("
66 | #if !defined(SND_LIB_VER) || (SND_LIB_VERSION < SND_LIB_VER(1, 2, 6))
| ^
SND_LIB_VER was only added in v1.2.5 so currently used distros run into
this. I've restructured to:
#ifdef SND_LIB_VER
#if SND_LIB_VERSION >= SND_LIB_VER(1, 2, 6)
#define LIB_HAS_LOAD_STRING
#endif
#endif
#ifndef LIB_HAS_LOAD_STRING
which is a bit ugly but splits the use of SND_LIB_VER into a separate if
statement which causes the preprocessor to do the right thing.
diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c index b87475fb7372..0f533707484c 100644 --- a/tools/testing/selftests/alsa/mixer-test.c +++ b/tools/testing/selftests/alsa/mixer-test.c @@ -46,22 +46,68 @@ struct ctl_data { struct ctl_data *next; }; +static const char *alsa_config = +"ctl.hw {\n" +" @args [ CARD ]\n" +" @args.CARD.type string\n" +" type hw\n" +" card $CARD\n" +"}\n" +; + int num_cards = 0; int num_controls = 0; struct card_data *card_list = NULL; struct ctl_data *ctl_list = NULL; +#if !defined(SND_LIB_VER) || SND_LIB_VERSION < SND_LIB_VER(1, 2, 6) +int snd_config_load_string(snd_config_t **config, const char *s, size_t size) +{ + snd_input_t *input; + snd_config_t *dst; + int err; + + assert(config && s); + if (size == 0) + size = strlen(s); + err = snd_input_buffer_open(&input, s, size); + if (err < 0) + return err; + err = snd_config_top(&dst); + if (err < 0) { + snd_input_close(input); + return err; + } + err = snd_config_load(dst, input); + snd_input_close(input); + if (err < 0) { + snd_config_delete(dst); + return err; + } + *config = dst; + return 0; +} +#endif + void find_controls(void) { char name[32]; int card, ctl, err; struct card_data *card_data; struct ctl_data *ctl_data; + snd_config_t *config; card = -1; if (snd_card_next(&card) < 0 || card < 0) return; + err = snd_config_load_string(&config, alsa_config, strlen(alsa_config)); + if (err < 0) { + ksft_print_msg("Unable to parse custom alsa-lib configuration: %s\n", + snd_strerror(err)); + ksft_exit_fail(); + } + while (card >= 0) { sprintf(name, "hw:%d", card); @@ -71,7 +117,7 @@ void find_controls(void) ksft_exit_fail(); } - err = snd_ctl_open(&card_data->handle, name, 0); + err = snd_ctl_open_lconf(&card_data->handle, name, 0, config); if (err < 0) { ksft_print_msg("Failed to get hctl for card %d: %s\n", card, snd_strerror(err)); @@ -147,6 +193,8 @@ void find_controls(void) break; } } + + snd_config_delete(config); } /*
As mentined by Takashi Sakamoto, the system-wide alsa-lib configuration may override the standard device declarations. This patch use the private alsa-lib configuration to set the predictable environment. Cc: Mark Brown <broonie@kernel.org> Cc: Shuah Khan <shuah@kernel.org> Cc: Takashi Sakamoto <o-takashi@sakamocchi.jp> Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> Cc: linux-kselftest@vger.kernel.org Link: https://lore.kernel.org/alsa-devel/Ya7TAHdMe9i41bsC@workstation/ Signed-off-by: Jaroslav Kysela <perex@perex.cz> --- tools/testing/selftests/alsa/mixer-test.c | 50 ++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-)