| Message ID | 1418337599-10239-1-git-send-email-benjamin.tissoires@redhat.com (mailing list archive) |
|---|---|
| State | Under Review, archived |
| Headers | show |
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; > } > >
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.
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 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
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; }
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(+)