[PATCH v2] HID: logitech-hidpp: prefix the name with Logitech

[PATCH v2] HID: logitech-hidpp: prefix the name with Logitech

[Benjamin Tissoires]

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.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

Changes in v2:
- renamed PREFIX_SIZE into PREFIX_LENGTH
- changed "name_length + PREFIX_LENGTH;" into "PREFIX_LENGTH + name_length;"
- rebased on Peter's last patch series

 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 2f420c0..274dbb7 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_LENGTH 9 /* "Logitech " */
+
+	int new_length;
+	char *new_name;
+
+	if (name_length > PREFIX_LENGTH &&
+	    strncmp(*name, "Logitech ", PREFIX_LENGTH) == 0)
+		/* The prefix has is already in the name */
+		return;
+
+	new_length = PREFIX_LENGTH + name_length;
+	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                                                        */
 /* -------------------------------------------------------------------------- */
@@ -321,6 +348,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;
 }
 
@@ -498,6 +529,9 @@ static char *hidpp_get_device_name(struct hidpp_device *hidpp)
 		index += ret;
 	}
 
+	/* include the terminating '\0' */
+	hidpp_prefix_name(&name, __name_length + 1);
+
 	return name;
 }
 
-- 
2.1.0

Re: [PATCH v2] HID: logitech-hidpp: prefix the name with Logitech

[Peter Wu]

On Thursday 11 December 2014 17:39:59 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.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Reviewed-by: Peter Wu <peter@lekensteyn.nl>

I have not tested this one, but the approach looks correct. What I have
also been thinking of is the possibility that Logitech adds "LOGITECH"
or "Logicool" (the Japanese trademark) before devices, but I think that
is unlikely so there is no need to check for other strings.

> ---
> 
> Changes in v2:
> - renamed PREFIX_SIZE into PREFIX_LENGTH
> - changed "name_length + PREFIX_LENGTH;" into "PREFIX_LENGTH + name_length;"
> - rebased on Peter's last patch series
> 
>  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 2f420c0..274dbb7 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_LENGTH 9 /* "Logitech " */
> +
> +	int new_length;
> +	char *new_name;
> +
> +	if (name_length > PREFIX_LENGTH &&
> +	    strncmp(*name, "Logitech ", PREFIX_LENGTH) == 0)
> +		/* The prefix has is already in the name */
> +		return;
> +
> +	new_length = PREFIX_LENGTH + name_length;
> +	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                                                        */
>  /* -------------------------------------------------------------------------- */
> @@ -321,6 +348,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;
>  }
>  
> @@ -498,6 +529,9 @@ static char *hidpp_get_device_name(struct hidpp_device *hidpp)
>  		index += ret;
>  	}
>  
> +	/* include the terminating '\0' */
> +	hidpp_prefix_name(&name, __name_length + 1);
> +
>  	return name;
>  }
>  
> 

-- 
Kind regards,
Peter
https://lekensteyn.nl

Re: [PATCH v2] HID: logitech-hidpp: prefix the name with Logitech

[Jiri Kosina]

On Thu, 11 Dec 2014, 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.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> 
> Changes in v2:
> - renamed PREFIX_SIZE into PREFIX_LENGTH
> - changed "name_length + PREFIX_LENGTH;" into "PREFIX_LENGTH + name_length;"
> - rebased on Peter's last patch series

Ok, looks reasonable. I was waiting whether someone from Logitech would 
comment on the prefix check that could be used other than "Logitech".

But we can add that later.

Applied to for-3.20/logitech, thanks.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2] HID: logitech-hidpp: prefix the name with Logitech

[Benjamin Tissoires]

On Dec 17 2014 or thereabouts, Jiri Kosina wrote:
> On Thu, 11 Dec 2014, 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.
> > 
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > ---
> > 
> > Changes in v2:
> > - renamed PREFIX_SIZE into PREFIX_LENGTH
> > - changed "name_length + PREFIX_LENGTH;" into "PREFIX_LENGTH + name_length;"
> > - rebased on Peter's last patch series
> 
> Ok, looks reasonable. I was waiting whether someone from Logitech would 
> comment on the prefix check that could be used other than "Logitech".
> 
> But we can add that later.
> 
> Applied to for-3.20/logitech, thanks.
> 

Hi Jiri,

Thanks for applying most of the patches (2 are missing, I'll raise them
in your inbox :-P )

Regarding this one, I was wondering if we could not force it into 3.19,
or at least add a stable@ tag. I had requested this in the first
submission, and the rationale was to not change a third time the name of
the device (from "Logitech Unifying Device. Wireless PID:XXXX" to
"[TMK]XXX" to "Logitech [TMK]XXX").

Userspace would be grateful to have a reliable name.

Cheers,
Benjamin

Re: [PATCH v2] HID: logitech-hidpp: prefix the name with Logitech

[Jiri Kosina]

On Wed, 17 Dec 2014, Benjamin Tissoires wrote:

> Thanks for applying most of the patches (2 are missing, I'll raise them
> in your inbox :-P )
> 
> Regarding this one, I was wondering if we could not force it into 3.19,
> or at least add a stable@ tag. I had requested this in the first
> submission, and the rationale was to not change a third time the name of
> the device (from "Logitech Unifying Device. Wireless PID:XXXX" to
> "[TMK]XXX" to "Logitech [TMK]XXX").
> 
> Userspace would be grateful to have a reliable name.

Fair enough. I've cherry-picked it from for-3.20/logitech into 
for-3.19/upstream-fixes as well.

Thanks,

-- 
Jiri Kosina
SUSE Labs