* [patch] HID: logitech-dj: small cleanup in rdcat()
@ 2013-10-19 9:08 Dan Carpenter
2013-10-19 9:46 ` walter harms
2013-10-30 13:45 ` Jiri Kosina
0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2013-10-19 9:08 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, kernel-janitors
We could pass the "rdsec" pointer instead of the address of the "rdesc"
and it's a little simpler.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 2e53024..a7947d8 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -542,9 +542,9 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
return 0;
}
-static void rdcat(char **rdesc, unsigned int *rsize, const char *data, unsigned int size)
+static void rdcat(char *rdesc, unsigned int *rsize, const char *data, unsigned int size)
{
- memcpy(*rdesc + *rsize, data, size);
+ memcpy(rdesc + *rsize, data, size);
*rsize += size;
}
@@ -567,31 +567,31 @@ static int logi_dj_ll_parse(struct hid_device *hid)
if (djdev->reports_supported & STD_KEYBOARD) {
dbg_hid("%s: sending a kbd descriptor, reports_supported: %x\n",
__func__, djdev->reports_supported);
- rdcat(&rdesc, &rsize, kbd_descriptor, sizeof(kbd_descriptor));
+ rdcat(rdesc, &rsize, kbd_descriptor, sizeof(kbd_descriptor));
}
if (djdev->reports_supported & STD_MOUSE) {
dbg_hid("%s: sending a mouse descriptor, reports_supported: "
"%x\n", __func__, djdev->reports_supported);
- rdcat(&rdesc, &rsize, mse_descriptor, sizeof(mse_descriptor));
+ rdcat(rdesc, &rsize, mse_descriptor, sizeof(mse_descriptor));
}
if (djdev->reports_supported & MULTIMEDIA) {
dbg_hid("%s: sending a multimedia report descriptor: %x\n",
__func__, djdev->reports_supported);
- rdcat(&rdesc, &rsize, consumer_descriptor, sizeof(consumer_descriptor));
+ rdcat(rdesc, &rsize, consumer_descriptor, sizeof(consumer_descriptor));
}
if (djdev->reports_supported & POWER_KEYS) {
dbg_hid("%s: sending a power keys report descriptor: %x\n",
__func__, djdev->reports_supported);
- rdcat(&rdesc, &rsize, syscontrol_descriptor, sizeof(syscontrol_descriptor));
+ rdcat(rdesc, &rsize, syscontrol_descriptor, sizeof(syscontrol_descriptor));
}
if (djdev->reports_supported & MEDIA_CENTER) {
dbg_hid("%s: sending a media center report descriptor: %x\n",
__func__, djdev->reports_supported);
- rdcat(&rdesc, &rsize, media_descriptor, sizeof(media_descriptor));
+ rdcat(rdesc, &rsize, media_descriptor, sizeof(media_descriptor));
}
if (djdev->reports_supported & KBD_LEDS) {
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [patch] HID: logitech-dj: small cleanup in rdcat()
2013-10-19 9:08 [patch] HID: logitech-dj: small cleanup in rdcat() Dan Carpenter
@ 2013-10-19 9:46 ` walter harms
2013-10-30 13:45 ` Jiri Kosina
1 sibling, 0 replies; 3+ messages in thread
From: walter harms @ 2013-10-19 9:46 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jiri Kosina, linux-input, kernel-janitors
Am 19.10.2013 11:08, schrieb Dan Carpenter:
> We could pass the "rdsec" pointer instead of the address of the "rdesc"
> and it's a little simpler.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 2e53024..a7947d8 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -542,9 +542,9 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
> return 0;
> }
>
> -static void rdcat(char **rdesc, unsigned int *rsize, const char *data, unsigned int size)
> +static void rdcat(char *rdesc, unsigned int *rsize, const char *data, unsigned int size)
> {
> - memcpy(*rdesc + *rsize, data, size);
> + memcpy(rdesc + *rsize, data, size);
> *rsize += size;
> }
That improves readability nicely, an other point is that the function hides the change of rsize.
IMHO it would be better either to return the new address or do the whole add on the caller
side:
suggestion 1:
rsize = rdcat();
suggestion 2:
rsize += rdcat();
Here you see immediately the new offset.
i know it is a matter of taste, it is just what i would do.
re,
wh
>
> @@ -567,31 +567,31 @@ static int logi_dj_ll_parse(struct hid_device *hid)
> if (djdev->reports_supported & STD_KEYBOARD) {
> dbg_hid("%s: sending a kbd descriptor, reports_supported: %x\n",
> __func__, djdev->reports_supported);
> - rdcat(&rdesc, &rsize, kbd_descriptor, sizeof(kbd_descriptor));
> + rdcat(rdesc, &rsize, kbd_descriptor, sizeof(kbd_descriptor));
> }
>
> if (djdev->reports_supported & STD_MOUSE) {
> dbg_hid("%s: sending a mouse descriptor, reports_supported: "
> "%x\n", __func__, djdev->reports_supported);
> - rdcat(&rdesc, &rsize, mse_descriptor, sizeof(mse_descriptor));
> + rdcat(rdesc, &rsize, mse_descriptor, sizeof(mse_descriptor));
> }
>
> if (djdev->reports_supported & MULTIMEDIA) {
> dbg_hid("%s: sending a multimedia report descriptor: %x\n",
> __func__, djdev->reports_supported);
> - rdcat(&rdesc, &rsize, consumer_descriptor, sizeof(consumer_descriptor));
> + rdcat(rdesc, &rsize, consumer_descriptor, sizeof(consumer_descriptor));
> }
>
> if (djdev->reports_supported & POWER_KEYS) {
> dbg_hid("%s: sending a power keys report descriptor: %x\n",
> __func__, djdev->reports_supported);
> - rdcat(&rdesc, &rsize, syscontrol_descriptor, sizeof(syscontrol_descriptor));
> + rdcat(rdesc, &rsize, syscontrol_descriptor, sizeof(syscontrol_descriptor));
> }
>
> if (djdev->reports_supported & MEDIA_CENTER) {
> dbg_hid("%s: sending a media center report descriptor: %x\n",
> __func__, djdev->reports_supported);
> - rdcat(&rdesc, &rsize, media_descriptor, sizeof(media_descriptor));
> + rdcat(rdesc, &rsize, media_descriptor, sizeof(media_descriptor));
> }
>
> if (djdev->reports_supported & KBD_LEDS) {
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [patch] HID: logitech-dj: small cleanup in rdcat()
2013-10-19 9:08 [patch] HID: logitech-dj: small cleanup in rdcat() Dan Carpenter
2013-10-19 9:46 ` walter harms
@ 2013-10-30 13:45 ` Jiri Kosina
1 sibling, 0 replies; 3+ messages in thread
From: Jiri Kosina @ 2013-10-30 13:45 UTC (permalink / raw)
To: Dan Carpenter; +Cc: linux-input, kernel-janitors
On Sat, 19 Oct 2013, Dan Carpenter wrote:
> We could pass the "rdsec" pointer instead of the address of the "rdesc"
> and it's a little simpler.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Cosmetic change on a borderline of being justified to bother with :)
Thanks Dan, applying now for 3.13.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-10-30 13:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-19 9:08 [patch] HID: logitech-dj: small cleanup in rdcat() Dan Carpenter
2013-10-19 9:46 ` walter harms
2013-10-30 13:45 ` Jiri Kosina
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).