Re: Zeroing the report before setting fields

[Pete Zaitcev <zaitcev@redhat.com>]

On Sat, 13 Mar 2010 23:25:11 +0100 (CET)
Jiri Kosina <jkosina@suse.cz> wrote:

> I don't think it's super important (basically nothing can be considered 
> super-hot path when dealing with such slow hardware as HID devices :) ), 
> but I wouldn't object to adding extra check before calling the memset.

I thought about the issue some more and came to the conclusion that
all the bit counting in my patch was entirely unnecessary (it's still
needed in the code that sets them, of course).

What would you say to the following? I tested it to work with the
affected keyboard. Seems like a simpler way to remove the overlap
and the trailing bits.

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 368fbb0..7f2a3b9 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -940,13 +940,8 @@ static void hid_output_field(struct hid_field *field, __u8 *data)
 	unsigned count = field->report_count;
 	unsigned offset = field->report_offset;
 	unsigned size = field->report_size;
-	unsigned bitsused = offset + count * size;
 	unsigned n;
 
-	/* make sure the unused bits in the last byte are zeros */
-	if (count > 0 && size > 0 && (bitsused % 8) != 0)
-		data[(bitsused-1)/8] &= (1 << (bitsused % 8)) - 1;
-
 	for (n = 0; n < count; n++) {
 		if (field->logical_minimum < 0)	/* signed values */
 			implement(data, offset + n * size, size, s32ton(field->value[n], size));
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 56d06cd..1332ed0 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -505,7 +505,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
 			return;
 		}
 
-		usbhid->out[usbhid->outhead].raw_report = kmalloc(len, GFP_ATOMIC);
+		usbhid->out[usbhid->outhead].raw_report = kzalloc(len, GFP_ATOMIC);
 		if (!usbhid->out[usbhid->outhead].raw_report) {
 			dev_warn(&hid->dev, "output queueing failed\n");
 			return;
@@ -537,7 +537,7 @@ static void __usbhid_submit_report(struct hid_device *hid, struct hid_report *re
 	}
 
 	if (dir == USB_DIR_OUT) {
-		usbhid->ctrl[usbhid->ctrlhead].raw_report = kmalloc(len, GFP_ATOMIC);
+		usbhid->ctrl[usbhid->ctrlhead].raw_report = kzalloc(len, GFP_ATOMIC);
 		if (!usbhid->ctrl[usbhid->ctrlhead].raw_report) {
 			dev_warn(&hid->dev, "control queueing failed\n");
 			return;

Greetings,
-- Pete