[PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Jaroslav Kysela]

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(-)

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);
 }
 
 /*
-- 
2.31.1

[PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Jaroslav Kysela]

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(-)

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);
 }
 
 /*
-- 
2.31.1

Re: [PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Mark Brown]

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

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>

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

Re: [PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Mark Brown]

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

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>

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

Re: [PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Takashi Sakamoto]

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

Re: [PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Mark Brown]

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

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.

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

Re: [PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Mark Brown]

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

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.

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

Re: [PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Jaroslav Kysela]

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

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Jaroslav Kysela]

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

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Mark Brown]

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

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.

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

Re: [PATCH] kselftest: alsa: Use private alsa-lib configuration in mixer test

[Mark Brown]

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

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.

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