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

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