[PATCH] winebus.sys: Do not report HID report read errors unconditionally

[Aric Stewart]

On 8/15/18 1:47 AM, Kai Krakow wrote:
> Hello!
> 
> 2018-08-15 8:10 GMT+02:00 Alexandre Julliard <julliard at winehq.org>:
>> Kai Krakow <kai at kaishome.de> writes:
>>
>>> Device reports may come in faster than our consumers could possibly read
>>> them, this is especially true for multi-axis events: When you move a
>>> stick across its range, it will always generate at least two events, one
>>> for the x axis, and one for the y axis. This is not really an error
>>> situation, so let's report this situation only once at most. If we
>>> already know the multi-event situation, let's skip logging this
>>> completely: We won't loose any information anyway because the report
>>> contains a complete device state and only axis positions were updated.
>>>
>>> Thus, this commit adds a parameter to process_hid_report() to let it
>>> know if we are currently processing reports that are known to be sent
>>> multiple times in sequence (with updated reports).
>>>
>>> Also, if our consumers are slow to respond, then report the issue only
>>> once and not per each occurrence during the duration of one consumer
>>> read cycle.
>>>
>>> Finally, this is not really an error, it's a warning at most, thus
>>> degrade the error message to a warning to not pollute peoples consoles
>>> and logs with unimportant stuff.
>>
>> Is it really necessary to add all that complexity then? Why not simply
>> remove the message?
> 
> I was also thinking about it but I'm not completely sure if there are
> valid cases for the warning to appear. As lined out, it may still
> loose button events if a reader in the queues doesn't read fast
> enough. And I don't know the complete driver stack in detail.
> 
> Discarding the message only for axis events is safe, tho. This is why
> the boolean parameter was added. The other change just reduces the log
> impact in case the warning would be displayed.
> 
> The problem here is that the intermediate layer needs to translate
> between HID events from Linux to full device reports for Windows, so
> it just coalesces events into the report state. But it's not a queue.
> Thus, having the warning may help debugging any problems left.
> 
> But if someone with more insight into the whole driver stack sees no
> more problems, I'm okay with removing the message completely.
> 
> @Aric: What do you think?
> 
> 
> Regards,
> Kai
> 

I have no strong opinion in that regard. It does seem like a lot of complexity to simply silence a message. I see no more problems with the driver stack and have no problem removing the message if Kai feels like it really is as harmless as it seems.

-aric