| Message ID | 20131019090859.GD9312@longonot.mountain (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
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 > -- 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 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.
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) {
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> -- 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