All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Takashi Iwai <tiwai@suse.de>, Jaroslav Kysela <perex@perex.cz>,
	Shuah Khan <shuah@kernel.org>
Cc: alsa-devel@alsa-project.org, linux-kselftest@vger.kernel.org,
	Mark Brown <broonie@kernel.org>
Subject: [PATCH v1 1/2] kselftest: alsa: Factor out check that values meet constraints
Date: Fri, 17 Dec 2021 13:02:12 +0000	[thread overview]
Message-ID: <20211217130213.3893415-2-broonie@kernel.org> (raw)
In-Reply-To: <20211217130213.3893415-1-broonie@kernel.org>

To simplify the code a bit and allow future reuse factor the checks that
values we read are valid out of test_ctl_get_value() into a separate
function which can be reused later. As part of this extend the test to
check all the values for the control, not just the first one.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/mixer-test.c | 141 +++++++++++++---------
 1 file changed, 82 insertions(+), 59 deletions(-)

diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c
index b798a76f6825..b009fc5df605 100644
--- a/tools/testing/selftests/alsa/mixer-test.c
+++ b/tools/testing/selftests/alsa/mixer-test.c
@@ -193,124 +193,147 @@ void find_controls(void)
 	snd_config_delete(config);
 }
 
-/*
- * Check that we can read the default value and it is valid. Write
- * tests use the read value to restore the default.
- */
-void test_ctl_get_value(struct ctl_data *ctl)
+bool ctl_value_index_valid(struct ctl_data *ctl, snd_ctl_elem_value_t *val,
+			   int index)
 {
-	int err;
 	long int_val;
 	long long int64_val;
 
-	/* If the control is turned off let's be polite */
-	if (snd_ctl_elem_info_is_inactive(ctl->info)) {
-		ksft_print_msg("%s is inactive\n", ctl->name);
-		ksft_test_result_skip("get_value.%d.%d\n",
-				      ctl->card->card, ctl->elem);
-		return;
-	}
-
-	/* Can't test reading on an unreadable control */
-	if (!snd_ctl_elem_info_is_readable(ctl->info)) {
-		ksft_print_msg("%s is not readable\n", ctl->name);
-		ksft_test_result_skip("get_value.%d.%d\n",
-				      ctl->card->card, ctl->elem);
-		return;
-	}
-
-	err = snd_ctl_elem_read(ctl->card->handle, ctl->def_val);
-	if (err < 0) {
-		ksft_print_msg("snd_ctl_elem_read() failed: %s\n",
-			       snd_strerror(err));
-		goto out;
-	}
-
 	switch (snd_ctl_elem_info_get_type(ctl->info)) {
 	case SND_CTL_ELEM_TYPE_NONE:
-		ksft_print_msg("%s Invalid control type NONE\n", ctl->name);
-		err = -1;
-		break;
+		ksft_print_msg("%s.%d Invalid control type NONE\n",
+			       ctl->name, index);
+		return false;
 
 	case SND_CTL_ELEM_TYPE_BOOLEAN:
-		int_val = snd_ctl_elem_value_get_boolean(ctl->def_val, 0);
+		int_val = snd_ctl_elem_value_get_boolean(val, index);
 		switch (int_val) {
 		case 0:
 		case 1:
 			break;
 		default:
-			ksft_print_msg("%s Invalid boolean value %ld\n",
-				       ctl->name, int_val);
-			err = -1;
-			break;
+			ksft_print_msg("%s.%d Invalid boolean value %ld\n",
+				       ctl->name, index, int_val);
+			return false;
 		}
 		break;
 
 	case SND_CTL_ELEM_TYPE_INTEGER:
-		int_val = snd_ctl_elem_value_get_integer(ctl->def_val, 0);
+		int_val = snd_ctl_elem_value_get_integer(val, index);
 
 		if (int_val < snd_ctl_elem_info_get_min(ctl->info)) {
-			ksft_print_msg("%s value %ld less than minimum %ld\n",
-				       ctl->name, int_val,
+			ksft_print_msg("%s.%d value %ld less than minimum %ld\n",
+				       ctl->name, index, int_val,
 				       snd_ctl_elem_info_get_min(ctl->info));
-			err = -1;
+			return false;
 		}
 
 		if (int_val > snd_ctl_elem_info_get_max(ctl->info)) {
-			ksft_print_msg("%s value %ld more than maximum %ld\n",
-				       ctl->name, int_val,
+			ksft_print_msg("%s.%d value %ld more than maximum %ld\n",
+				       ctl->name, index, int_val,
 				       snd_ctl_elem_info_get_max(ctl->info));
-			err = -1;
+			return false;
 		}
 
 		/* Only check step size if there is one and we're in bounds */
-		if (err >= 0 && snd_ctl_elem_info_get_step(ctl->info) &&
+		if (snd_ctl_elem_info_get_step(ctl->info) &&
 		    (int_val - snd_ctl_elem_info_get_min(ctl->info) %
 		     snd_ctl_elem_info_get_step(ctl->info))) {
-			ksft_print_msg("%s value %ld invalid for step %ld minimum %ld\n",
-				       ctl->name, int_val,
+			ksft_print_msg("%s.%d value %ld invalid for step %ld minimum %ld\n",
+				       ctl->name, index, int_val,
 				       snd_ctl_elem_info_get_step(ctl->info),
 				       snd_ctl_elem_info_get_min(ctl->info));
-			err = -1;
+			return false;
 		}
 		break;
 
 	case SND_CTL_ELEM_TYPE_INTEGER64:
-		int64_val = snd_ctl_elem_value_get_integer64(ctl->def_val, 0);
+		int64_val = snd_ctl_elem_value_get_integer64(val, index);
 
 		if (int64_val < snd_ctl_elem_info_get_min64(ctl->info)) {
-			ksft_print_msg("%s value %lld less than minimum %lld\n",
-				       ctl->name, int64_val,
+			ksft_print_msg("%s.%d value %lld less than minimum %lld\n",
+				       ctl->name, index, int64_val,
 				       snd_ctl_elem_info_get_min64(ctl->info));
-			err = -1;
+			return false;
 		}
 
 		if (int64_val > snd_ctl_elem_info_get_max64(ctl->info)) {
-			ksft_print_msg("%s value %lld more than maximum %lld\n",
-				       ctl->name, int64_val,
+			ksft_print_msg("%s.%d value %lld more than maximum %lld\n",
+				       ctl->name, index, int64_val,
 				       snd_ctl_elem_info_get_max(ctl->info));
-			err = -1;
+			return false;
 		}
 
 		/* Only check step size if there is one and we're in bounds */
-		if (err >= 0 && snd_ctl_elem_info_get_step64(ctl->info) &&
+		if (snd_ctl_elem_info_get_step64(ctl->info) &&
 		    (int64_val - snd_ctl_elem_info_get_min64(ctl->info)) %
 		    snd_ctl_elem_info_get_step64(ctl->info)) {
-			ksft_print_msg("%s value %lld invalid for step %lld minimum %lld\n",
-				       ctl->name, int64_val,
+			ksft_print_msg("%s.%d value %lld invalid for step %lld minimum %lld\n",
+				       ctl->name, index, int64_val,
 				       snd_ctl_elem_info_get_step64(ctl->info),
 				       snd_ctl_elem_info_get_min64(ctl->info));
-			err = -1;
+			return false;
 		}
 		break;
 
 	default:
 		/* No tests for other types */
+		break;
+	}
+
+	return true;
+}
+
+/*
+ * Check that the provided value meets the constraints for the
+ * provided control.
+ */
+bool ctl_value_valid(struct ctl_data *ctl, snd_ctl_elem_value_t *val)
+{
+	int i;
+	bool valid = true;
+
+	for (i = 0; i < snd_ctl_elem_info_get_count(ctl->info); i++)
+		if (!ctl_value_index_valid(ctl, val, i))
+			valid = false;
+
+	return valid;
+}
+
+/*
+ * Check that we can read the default value and it is valid. Write
+ * tests use the read value to restore the default.
+ */
+void test_ctl_get_value(struct ctl_data *ctl)
+{
+	int err;
+
+	/* If the control is turned off let's be polite */
+	if (snd_ctl_elem_info_is_inactive(ctl->info)) {
+		ksft_print_msg("%s is inactive\n", ctl->name);
+		ksft_test_result_skip("get_value.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	/* Can't test reading on an unreadable control */
+	if (!snd_ctl_elem_info_is_readable(ctl->info)) {
+		ksft_print_msg("%s is not readable\n", ctl->name);
 		ksft_test_result_skip("get_value.%d.%d\n",
 				      ctl->card->card, ctl->elem);
 		return;
 	}
 
+	err = snd_ctl_elem_read(ctl->card->handle, ctl->def_val);
+	if (err < 0) {
+		ksft_print_msg("snd_ctl_elem_read() failed: %s\n",
+			       snd_strerror(err));
+		goto out;
+	}
+
+	if (!ctl_value_valid(ctl, ctl->def_val))
+		err = -EINVAL;
+
 out:
 	ksft_test_result(err >= 0, "get_value.%d.%d\n",
 			 ctl->card->card, ctl->elem);
-- 
2.30.2


WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org>
To: Takashi Iwai <tiwai@suse.de>, Jaroslav Kysela <perex@perex.cz>,
	Shuah Khan <shuah@kernel.org>
Cc: alsa-devel@alsa-project.org, Mark Brown <broonie@kernel.org>,
	linux-kselftest@vger.kernel.org
Subject: [PATCH v1 1/2] kselftest: alsa: Factor out check that values meet constraints
Date: Fri, 17 Dec 2021 13:02:12 +0000	[thread overview]
Message-ID: <20211217130213.3893415-2-broonie@kernel.org> (raw)
In-Reply-To: <20211217130213.3893415-1-broonie@kernel.org>

To simplify the code a bit and allow future reuse factor the checks that
values we read are valid out of test_ctl_get_value() into a separate
function which can be reused later. As part of this extend the test to
check all the values for the control, not just the first one.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/alsa/mixer-test.c | 141 +++++++++++++---------
 1 file changed, 82 insertions(+), 59 deletions(-)

diff --git a/tools/testing/selftests/alsa/mixer-test.c b/tools/testing/selftests/alsa/mixer-test.c
index b798a76f6825..b009fc5df605 100644
--- a/tools/testing/selftests/alsa/mixer-test.c
+++ b/tools/testing/selftests/alsa/mixer-test.c
@@ -193,124 +193,147 @@ void find_controls(void)
 	snd_config_delete(config);
 }
 
-/*
- * Check that we can read the default value and it is valid. Write
- * tests use the read value to restore the default.
- */
-void test_ctl_get_value(struct ctl_data *ctl)
+bool ctl_value_index_valid(struct ctl_data *ctl, snd_ctl_elem_value_t *val,
+			   int index)
 {
-	int err;
 	long int_val;
 	long long int64_val;
 
-	/* If the control is turned off let's be polite */
-	if (snd_ctl_elem_info_is_inactive(ctl->info)) {
-		ksft_print_msg("%s is inactive\n", ctl->name);
-		ksft_test_result_skip("get_value.%d.%d\n",
-				      ctl->card->card, ctl->elem);
-		return;
-	}
-
-	/* Can't test reading on an unreadable control */
-	if (!snd_ctl_elem_info_is_readable(ctl->info)) {
-		ksft_print_msg("%s is not readable\n", ctl->name);
-		ksft_test_result_skip("get_value.%d.%d\n",
-				      ctl->card->card, ctl->elem);
-		return;
-	}
-
-	err = snd_ctl_elem_read(ctl->card->handle, ctl->def_val);
-	if (err < 0) {
-		ksft_print_msg("snd_ctl_elem_read() failed: %s\n",
-			       snd_strerror(err));
-		goto out;
-	}
-
 	switch (snd_ctl_elem_info_get_type(ctl->info)) {
 	case SND_CTL_ELEM_TYPE_NONE:
-		ksft_print_msg("%s Invalid control type NONE\n", ctl->name);
-		err = -1;
-		break;
+		ksft_print_msg("%s.%d Invalid control type NONE\n",
+			       ctl->name, index);
+		return false;
 
 	case SND_CTL_ELEM_TYPE_BOOLEAN:
-		int_val = snd_ctl_elem_value_get_boolean(ctl->def_val, 0);
+		int_val = snd_ctl_elem_value_get_boolean(val, index);
 		switch (int_val) {
 		case 0:
 		case 1:
 			break;
 		default:
-			ksft_print_msg("%s Invalid boolean value %ld\n",
-				       ctl->name, int_val);
-			err = -1;
-			break;
+			ksft_print_msg("%s.%d Invalid boolean value %ld\n",
+				       ctl->name, index, int_val);
+			return false;
 		}
 		break;
 
 	case SND_CTL_ELEM_TYPE_INTEGER:
-		int_val = snd_ctl_elem_value_get_integer(ctl->def_val, 0);
+		int_val = snd_ctl_elem_value_get_integer(val, index);
 
 		if (int_val < snd_ctl_elem_info_get_min(ctl->info)) {
-			ksft_print_msg("%s value %ld less than minimum %ld\n",
-				       ctl->name, int_val,
+			ksft_print_msg("%s.%d value %ld less than minimum %ld\n",
+				       ctl->name, index, int_val,
 				       snd_ctl_elem_info_get_min(ctl->info));
-			err = -1;
+			return false;
 		}
 
 		if (int_val > snd_ctl_elem_info_get_max(ctl->info)) {
-			ksft_print_msg("%s value %ld more than maximum %ld\n",
-				       ctl->name, int_val,
+			ksft_print_msg("%s.%d value %ld more than maximum %ld\n",
+				       ctl->name, index, int_val,
 				       snd_ctl_elem_info_get_max(ctl->info));
-			err = -1;
+			return false;
 		}
 
 		/* Only check step size if there is one and we're in bounds */
-		if (err >= 0 && snd_ctl_elem_info_get_step(ctl->info) &&
+		if (snd_ctl_elem_info_get_step(ctl->info) &&
 		    (int_val - snd_ctl_elem_info_get_min(ctl->info) %
 		     snd_ctl_elem_info_get_step(ctl->info))) {
-			ksft_print_msg("%s value %ld invalid for step %ld minimum %ld\n",
-				       ctl->name, int_val,
+			ksft_print_msg("%s.%d value %ld invalid for step %ld minimum %ld\n",
+				       ctl->name, index, int_val,
 				       snd_ctl_elem_info_get_step(ctl->info),
 				       snd_ctl_elem_info_get_min(ctl->info));
-			err = -1;
+			return false;
 		}
 		break;
 
 	case SND_CTL_ELEM_TYPE_INTEGER64:
-		int64_val = snd_ctl_elem_value_get_integer64(ctl->def_val, 0);
+		int64_val = snd_ctl_elem_value_get_integer64(val, index);
 
 		if (int64_val < snd_ctl_elem_info_get_min64(ctl->info)) {
-			ksft_print_msg("%s value %lld less than minimum %lld\n",
-				       ctl->name, int64_val,
+			ksft_print_msg("%s.%d value %lld less than minimum %lld\n",
+				       ctl->name, index, int64_val,
 				       snd_ctl_elem_info_get_min64(ctl->info));
-			err = -1;
+			return false;
 		}
 
 		if (int64_val > snd_ctl_elem_info_get_max64(ctl->info)) {
-			ksft_print_msg("%s value %lld more than maximum %lld\n",
-				       ctl->name, int64_val,
+			ksft_print_msg("%s.%d value %lld more than maximum %lld\n",
+				       ctl->name, index, int64_val,
 				       snd_ctl_elem_info_get_max(ctl->info));
-			err = -1;
+			return false;
 		}
 
 		/* Only check step size if there is one and we're in bounds */
-		if (err >= 0 && snd_ctl_elem_info_get_step64(ctl->info) &&
+		if (snd_ctl_elem_info_get_step64(ctl->info) &&
 		    (int64_val - snd_ctl_elem_info_get_min64(ctl->info)) %
 		    snd_ctl_elem_info_get_step64(ctl->info)) {
-			ksft_print_msg("%s value %lld invalid for step %lld minimum %lld\n",
-				       ctl->name, int64_val,
+			ksft_print_msg("%s.%d value %lld invalid for step %lld minimum %lld\n",
+				       ctl->name, index, int64_val,
 				       snd_ctl_elem_info_get_step64(ctl->info),
 				       snd_ctl_elem_info_get_min64(ctl->info));
-			err = -1;
+			return false;
 		}
 		break;
 
 	default:
 		/* No tests for other types */
+		break;
+	}
+
+	return true;
+}
+
+/*
+ * Check that the provided value meets the constraints for the
+ * provided control.
+ */
+bool ctl_value_valid(struct ctl_data *ctl, snd_ctl_elem_value_t *val)
+{
+	int i;
+	bool valid = true;
+
+	for (i = 0; i < snd_ctl_elem_info_get_count(ctl->info); i++)
+		if (!ctl_value_index_valid(ctl, val, i))
+			valid = false;
+
+	return valid;
+}
+
+/*
+ * Check that we can read the default value and it is valid. Write
+ * tests use the read value to restore the default.
+ */
+void test_ctl_get_value(struct ctl_data *ctl)
+{
+	int err;
+
+	/* If the control is turned off let's be polite */
+	if (snd_ctl_elem_info_is_inactive(ctl->info)) {
+		ksft_print_msg("%s is inactive\n", ctl->name);
+		ksft_test_result_skip("get_value.%d.%d\n",
+				      ctl->card->card, ctl->elem);
+		return;
+	}
+
+	/* Can't test reading on an unreadable control */
+	if (!snd_ctl_elem_info_is_readable(ctl->info)) {
+		ksft_print_msg("%s is not readable\n", ctl->name);
 		ksft_test_result_skip("get_value.%d.%d\n",
 				      ctl->card->card, ctl->elem);
 		return;
 	}
 
+	err = snd_ctl_elem_read(ctl->card->handle, ctl->def_val);
+	if (err < 0) {
+		ksft_print_msg("snd_ctl_elem_read() failed: %s\n",
+			       snd_strerror(err));
+		goto out;
+	}
+
+	if (!ctl_value_valid(ctl, ctl->def_val))
+		err = -EINVAL;
+
 out:
 	ksft_test_result(err >= 0, "get_value.%d.%d\n",
 			 ctl->card->card, ctl->elem);
-- 
2.30.2


  reply	other threads:[~2021-12-17 13:03 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-17 13:02 [PATCH v1 0/2] kselftest: alsa: Small enhancements to mixer-test Mark Brown
2021-12-17 13:02 ` Mark Brown
2021-12-17 13:02 ` Mark Brown [this message]
2021-12-17 13:02   ` [PATCH v1 1/2] kselftest: alsa: Factor out check that values meet constraints Mark Brown
2021-12-17 13:32   ` Cezary Rojewski
2021-12-17 13:32     ` Cezary Rojewski
2021-12-17 14:38     ` Mark Brown
2021-12-17 14:38       ` Mark Brown
2021-12-17 14:54       ` Cezary Rojewski
2021-12-17 14:54         ` Cezary Rojewski
2021-12-17 13:02 ` [PATCH v1 2/2] kselftest: alsa: Validate values read from enumerations Mark Brown
2021-12-17 13:02   ` Mark Brown
2021-12-17 15:01 ` [PATCH v1 0/2] kselftest: alsa: Small enhancements to mixer-test Cezary Rojewski
2021-12-17 15:01   ` Cezary Rojewski
2021-12-25  8:13 ` Takashi Iwai
2021-12-25  8:13   ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211217130213.3893415-2-broonie@kernel.org \
    --to=broonie@kernel.org \
    --cc=alsa-devel@alsa-project.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=shuah@kernel.org \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.