Re: [PATCH] selftests: alsa: Start validating control names

[Takashi Iwai <tiwai@suse.de>]

On Wed, 20 Apr 2022 22:33:20 +0200,
Mark Brown wrote:
> 
> +bool strend(const char *haystack, const char *needle)

Missing static?

> +{
> +	size_t haystack_len = strlen(haystack);
> +	size_t needle_len = strlen(needle);
> +
> +	if (needle_len > haystack_len)
> +		return false;
> +	return strcmp(haystack + haystack_len - needle_len, needle) == 0;
> +}
> +
> +static void test_ctl_name(struct ctl_data *ctl)
> +{
> +	bool name_ok = true;
> +	bool check;
> +
> +	/* Only boolean controls should end in Switch */
> +	if (strend(ctl->name, "Switch")) {

This should be with " Switch" so that it won't check a concatenated
word.

> +		if (snd_ctl_elem_info_get_type(ctl->info) != SND_CTL_ELEM_TYPE_BOOLEAN) {
> +			ksft_print_msg("%d.%d %s ends in Switch but is not boolean\n",
> +				       ctl->card->card, ctl->elem, ctl->name);
> +			name_ok = false;
> +		}
> +	}
> +
> +	/* Writeable boolean controls should end in Switch */
> +	if (snd_ctl_elem_info_get_type(ctl->info) == SND_CTL_ELEM_TYPE_BOOLEAN &&
> +	    snd_ctl_elem_info_is_writable(ctl->info)) {
> +		if (!strend(ctl->name, "Switch")) {
> +			ksft_print_msg("%d.%d %s is a writeable boolean but not a Switch\n",
> +				       ctl->card->card, ctl->elem, ctl->name);
> +			name_ok = false;

I'm afraid that this would hit too many when applying to the existing
code; although the control name should be indeed with Switch suffix,
we tend to allow without suffix for casual non-standard elements.

But having the check would help for avoiding such a mistake for the
future code, so it's fine to add this strict check, IMO.


thanks,

Takashi