linux-kernel - Re: [PATCH 2/2] HID: logitech-hidpp: prefix the name with Logitech

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1511661.CjWHljXQbp@al>
Date:	Thu, 11 Dec 2014 00:01:27 +0100
From:	Peter Wu <peter@...ensteyn.nl>
To:	Benjamin Tissoires <benjamin.tissoires@...hat.com>
Cc:	Jiri Kosina <jkosina@...e.cz>,
	Nestor Lopez Casado <nlopezcasad@...itech.com>,
	Peter Hutterer <peter.hutterer@...-t.net>,
	linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] HID: logitech-hidpp: prefix the name with Logitech

On Wednesday 10 December 2014 17:21:10 Benjamin Tissoires wrote:
> Current names are reported as "K750", "M705", and it can be misleading
> for the users when they look at their input device list.
> 
> Prefixing the names with "Logitech " makes things better.

Doesn't this apply to all input devices? Like, every USB device can be
queried for the manufacturer which could then by included by the input
subsystem.

What about duplicate names? Can this patch cause a conflict with devices
having the same name?

> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> ---
> 
> Hi Jiri,
> 
> I'd love to see this one in 3.19 (after a strong review, of course).
> The idea came with the mouse DPI database that Peter is currently working on
> (see http://who-t.blogspot.com.au/2014/12/building-a-dpi-database-for-mice.html).
> 
> I think, if you do not qualify the series for 3.19, we should drop it entirely.
> 3.19 introduces the hidpp module and changes the labels. The DPI hwdb will check
> on the label to match the actual mouse, so if this one does not get into 3.19,
> we will end up in 3 entries for each Logitech device.
> 
> Cheers,
> Benjamin
> 
> 
>  drivers/hid/hid-logitech-hidpp.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 3846305..8cf4352 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -282,6 +282,33 @@ static inline bool hidpp_report_is_connect_event(struct hidpp_report *report)
>  		(report->rap.sub_id == 0x41);
>  }
>  
> +/**
> + * hidpp_prefix_name() prefixes the current given name with "Logitech ".
> + */
> +static void hidpp_prefix_name(char **name, int name_length)
> +{
> +#define PREFIX_SIZE 9 /* "Logitech " */
> +
> +	int new_length;
> +	char *new_name;
> +
> +	if (name_length > PREFIX_SIZE &&
> +	    strncmp(*name, "Logitech ", PREFIX_SIZE) == 0)

I think you meant || here, not &&.

> +		/* The prefix has is already in the name */
> +		return;
> +
> +	new_length = name_length + PREFIX_SIZE;

Stylistic note, but 'PREFIX_SIZE + name_length' would match the order of
the string that gets prepended.

Kind regards,
Peter

> +	new_name = kzalloc(new_length, GFP_KERNEL);
> +	if (!new_name)
> +		return;
> +
> +	snprintf(new_name, new_length, "Logitech %s", *name);
> +
> +	kfree(*name);
> +
> +	*name = new_name;
> +}
> +
>  /* -------------------------------------------------------------------------- */
>  /* HIDP++ 1.0 commands                                                        */
>  /* -------------------------------------------------------------------------- */
> @@ -318,6 +345,10 @@ static char *hidpp_get_unifying_name(struct hidpp_device *hidpp_dev)
>  		return NULL;
>  
>  	memcpy(name, &response.rap.params[2], len);
> +
> +	/* include the terminating '\0' */
> +	hidpp_prefix_name(&name, len + 1);
> +
>  	return name;
>  }
>  
> @@ -489,6 +520,9 @@ static char *hidpp_get_device_name(struct hidpp_device *hidpp)
>  			feature_index, index, name + index,
>  			__name_length - index);
>  
> +	/* include the terminating '\0' */
> +	hidpp_prefix_name(&name, __name_length + 1);
> +
>  	return name;
>  
>  out_err:
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/