Incorrect processing of "1" raw_event() return value inside hid_input_report()

[Dmitry Semyonov]

Hello Jiri,

Please, refer to the commit b1a1442a23776756b254b69786848a94d92445ba
and to the second part of
http://thread.gmane.org/gmane.linux.kernel.input/39802. (The message
author was right but hesitated to insist. Therefore, I have to re-open
the issue):

>>         if (hdrv && hdrv->raw_event && hid_match_report(hid, report)) {
>>                 ret = hdrv->raw_event(hid, report, data, size);
>> -               if (ret != 0) {
>> +               if (ret < 0) {
>>                         ret = ret < 0 ? ret : 0;
>>                         goto unlock;
>>                 }
>> --------------------------------------------------
>>
>> So now it does not matter whether the raw_event method returns 0 or 1.
>> The documentation says:

(The hid_driver structure documentation in linux/hid.h)

>>     raw_event and event should return 0 on no action performed, 1 when
>>     no further processing should be done and negative on error
 [...]
> This change was done so that we don't call hid_report_raw_event() when
> ->raw_event() callback encountered error during parsing (and therefore
> returned negative value).

I cannot accept this answer because the processing of negative return
values was not affected by the above change.

What is worse, I cannot accept the change itself because it breaks the
contract established by the hid_driver structure documentation for HID
drivers.

Ubuntu guys had to revert it[1] due to changed behavior that uncovered
another bug[2]. A colleague of mine re-discovered the same root cause
while investigating a different issue, and had to revert the change as
well because of some existing proprietary keypad driver was using the
"1" value to indicate that no further processing is needed, (exactly
as specified in the documentation).

[1]https://lists.ubuntu.com/archives/kernel-team/2013-October/032351.html
[2]http://www.kernelhub.org/?p=2&msg=380901

A lot of HID drivers in the kernel source tree use negative, 0, and 1
return values in their *_raw_event() functions. Some of them may be
using the values incorrectly but at least they were verified by the
original developers, and I doubt you had a chance to properly re-test
*all* the drivers before introducing the breaking API change.
Therefore, I strongly recommend to revert the following two related
patches, and fix the specific driver(s) instead if there are any
issues left:

b1a1442a HID: core: fix reporting of raw events
556483e2 HID: remove self-assignment from hid_input_report

-- 
...Bye..Dmitry.