Re: [PATCH] v4l-utils: fix invalid protocol in streamzap keymap

[Matthias Reichl <hias@horus.com>]

On Sat, Feb 18, 2017 at 07:24:43PM +0000, Sean Young wrote:
> On Fri, Feb 17, 2017 at 10:19:16AM +0100, Matthias Reichl wrote:
> > ir-keytable can't load the streamzap keymap because the
> > protocol type RC5_SZ is invalid:
> > 
> > ./ir-keytable -w rc_keymaps/streamzap
> > Protocol RC5_SZ invalid
> > ...
> > 
> > Fix this by changing the protocol type to RC-5-SZ which
> > matches the kernel protocol rc-5-sz
> > 
> > Signed-off-by: Matthias Reichl <hias@horus.com>
> > ---
> >  utils/keytable/rc_keymaps/streamzap | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/utils/keytable/rc_keymaps/streamzap b/utils/keytable/rc_keymaps/streamzap
> > index 3512cd8..03d2cb8 100644
> > --- a/utils/keytable/rc_keymaps/streamzap
> > +++ b/utils/keytable/rc_keymaps/streamzap
> 
> This is file is generated by utils/keytable/gen_keytables.pl, so there is no
> use in patching it.

Ouch, I totally missed that. Thanks for pointing it out!

> Actually I think a better solution would be to be less pernickety about how
> the protocol is specified. ir-ctl also does this. How about the following
> patch.

I agree, this is a much better solution. It simplifies the protocol_map
and being more tolerant about spelling is also a benefit to the user.

> Sean
> 
> From: Sean Young <sean@mess.org>
> Subject: [PATCH] [PATCH v4l-utils] ir-keytable: be more permissive on protocol
>  name
> 
> Allowed the protocol to be specified with or without underscores or
> dashes. This also solves the problem of not being able to load
> the streamzap keymap.
> 
> ./ir-keytable -w rc_keymaps/streamzap
> Protocol RC5_SZ invalid
> 
> Reported-by: Matthias Reichl <hias@horus.com>
> Signed-off-by: Sean Young <sean@mess.org>
> ---
>  utils/keytable/keytable.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/utils/keytable/keytable.c b/utils/keytable/keytable.c
> index a6ecc9e..a35db5b 100644
> --- a/utils/keytable/keytable.c
> +++ b/utils/keytable/keytable.c
> @@ -120,9 +120,7 @@ const struct protocol_map_entry protocol_map[] = {
>  	{ "other",	NULL,		SYSFS_OTHER	},
>  	{ "lirc",	NULL,		SYSFS_LIRC	},
>  	{ "rc-5",	"/rc5_decoder",	SYSFS_RC5	},
> -	{ "rc5",	NULL,		SYSFS_RC5	},
>  	{ "rc-5x",	NULL,		SYSFS_INVALID	},
> -	{ "rc5x",	NULL,		SYSFS_INVALID	},
>  	{ "rc-5-sz",	NULL,		SYSFS_RC5_SZ	},
>  	{ "jvc",	"/jvc_decoder",	SYSFS_JVC	},
>  	{ "sony",	"/sony_decoder",SYSFS_SONY	},
> @@ -134,7 +132,6 @@ const struct protocol_map_entry protocol_map[] = {
>  	{ "mce_kbd",	NULL,		SYSFS_MCE_KBD	},
>  	{ "mce-kbd",	NULL,		SYSFS_MCE_KBD	},
>  	{ "rc-6",	"/rc6_decoder",	SYSFS_RC6	},
> -	{ "rc6",	NULL,		SYSFS_RC6	},
>  	{ "rc-6-0",	NULL,		SYSFS_INVALID	},
>  	{ "rc-6-6a-20",	NULL,		SYSFS_INVALID	},
>  	{ "rc-6-6a-24",	NULL,		SYSFS_INVALID	},
> @@ -145,6 +142,21 @@ const struct protocol_map_entry protocol_map[] = {
>  	{ NULL,		NULL,		SYSFS_INVALID	},
>  };
>  
> +static bool str_like(const char *a, const char *b)
> +{
> +	while (*a && *b) {
> +		if (*a == '-' || *a == '_')
> +			a++;
> +		if (*b == '-' || *b == '_')
> +			b++;
> +		if (tolower(*a) != tolower(*b))
> +			return false;
> +		a++; b++;
> +	}

A small nit: this code will fail if both strings are identical
and end with a dash or underscore. In that case we'll iterate
beyond then end of the strings.

Adding a continue after the dash/underscore increment should fix
this, then we won't increment the pointer by 2 within a loop

		if (*a == '-' || *a == '_') {
			a++;
			continue;
		}
		if (*b == '-' || *b == '_') {
			b++;
			continue;
		}

Other than that the patch looks fine to me and worked well.

> +
> +	return !*a && !*b;
> +}
> +
>  static enum sysfs_protocols parse_sysfs_protocol(const char *name, bool all_allowed)
>  {
>  	const struct protocol_map_entry *pme;
> @@ -156,7 +168,7 @@ static enum sysfs_protocols parse_sysfs_protocol(const char *name, bool all_allo
>  		return ~0;
>  
>  	for (pme = protocol_map; pme->name; pme++) {
> -		if (!strcasecmp(name, pme->name))
> +		if (str_like(name, pme->name))
>  			return pme->sysfs_protocol;
>  	}
>  
> -- 
> 2.9.3
>