Re: Zeroing the report before setting fields

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 22 Mar 2010, Pete Zaitcev wrote:

> > >  	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");
> 
> > Frankly, I like the generic aproach more ... if we were to add more 
> > low-level transport HID implementations later, I am sure this will be 
> > forgotten multiple times :) (as it isn't directly implied by the HID 
> > specification, but is rather a workaround for devices that are violating 
> > the spec by looking at the bits they shouldn't care about).
> 
> You are right, I screwed the pooch on the layering. Sorry.
> 
> Still, I do not want to go back to counting of field lengths.
> Too many what-ifs.
> 
> How about this (I tested it but clearly my tests are limited):
> 
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 368fbb0..e290bb3 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));
> @@ -966,6 +961,7 @@ void hid_output_report(struct hid_report *report, __u8 *data)
>  	if (report->id > 0)
>  		*data++ = report->id;
>  
> +	memset(data, 0, ((report->size - 1) >> 3) + 1);
>  	for (n = 0; n < report->maxfield; n++)
>  		hid_output_field(report->field[n], data);
>  }

This looks basically fine. Could you please send it with chagelog and your 
Signed-off-by entry?

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux