linux-kernel - Re: [PATCH v2 03/11] HID: hid-input: add usage_index argument in input

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAN+gG=GiAsn-5kefjRj_xgFfbuEXxj_PL7PaPES05wiZs+J-Ug@mail.gmail.com>
Date:	Tue, 6 Nov 2012 14:56:01 +0100
From:	Benjamin Tissoires <benjamin.tissoires@...il.com>
To:	Henrik Rydberg <rydberg@...omail.se>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Jiri Kosina <jkosina@...e.cz>,
	Stephane Chatty <chatty@...c.fr>,
	linux-input <linux-input@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 03/11] HID: hid-input: add usage_index argument in
 input_mapping and event.

On Tue, Oct 30, 2012 at 11:09 AM, Benjamin Tissoires
<benjamin.tissoires@...il.com> wrote:
> On Mon, Oct 29, 2012 at 8:25 PM, Henrik Rydberg <rydberg@...omail.se> wrote:
>> Hi Benjamin,
>>
>>> Currently, there is no way to know the index of the current field
>>> in the .input_mapping and .event callbacks  when this field is inside
>>> an array of HID fields.
>>> This patch forwards this index to the input_mapping and event
>>> callbacks.
>>
>> I agree with the idea, but the function argument list is becoming
>> ridiculously long... Could we remove the usage pointer argument, at
>> least?
>
> yeah, totally agree. Let me just check whether it will not introduce
> more problems than it solves for my driver.

Well, after a deeper look, it's a really bad idea. Every drivers that
implements input_mapping, input_mapped, event use at least once the
struct usage. So this would require change nearly every hid driver.

To solve that, and to minimize the impact on the other drivers, I'm
going to add a field in struct usage with the appropriate index.

Cheers,
Benjamin

>
>>
>>         int (*event)(struct hid_device *hdev, struct hid_field *field,
>>                         unsigned int usage_index, __s32 value);
>>
>>
>>> @@ -1071,19 +1072,24 @@ static void hid_input_field(struct hid_device *hid, struct hid_field *field,
>>>       for (n = 0; n < count; n++) {
>>>
>>>               if (HID_MAIN_ITEM_VARIABLE & field->flags) {
>>> -                     hid_process_event(hid, field, &field->usage[n], value[n], interrupt);
>>> +                     hid_process_event(hid, field, &field->usage[n], n,
>>> +                             value[n], interrupt);
>>>                       continue;
>>>               }
>>>
>>>               if (field->value[n] >= min && field->value[n] <= max
>>>                       && field->usage[field->value[n] - min].hid
>>>                       && search(value, field->value[n], count))
>>> -                             hid_process_event(hid, field, &field->usage[field->value[n] - min], 0, interrupt);
>>> +                             hid_process_event(hid, field,
>>> +                                     &field->usage[field->value[n] - min], n,
>>> +                                     0, interrupt);
>>
>> Wrong index?
>
> oops, I'll have to check that.
>
> Thanks,
> Benjamin
>
>>
>>>
>>>               if (value[n] >= min && value[n] <= max
>>>                       && field->usage[value[n] - min].hid
>>>                       && search(field->value, value[n], count))
>>> -                             hid_process_event(hid, field, &field->usage[value[n] - min], 1, interrupt);
>>> +                             hid_process_event(hid, field,
>>> +                                     &field->usage[value[n] - min], n,
>>> +                                     1, interrupt);
>>
>> Wrong index?
>>
>>>       }
>>>
>>>       memcpy(field->value, value, count * sizeof(__s32));
>>
>> Thanks,
>> Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/