https://bugs.winehq.org/show_bug.cgi?id=43125
--- Comment #7 from Kai Krakow kai@kaishome.de --- (In reply to Konrad Rzepecki from comment #6)
This code is in critical section and only setting ext->last_report_read to FALSE is after check. So other code should not affect this at all. Only possibility I see is that this "while" is never entered so TRUE is not set. I think this line with TRUE set should be moved outside while for two reasons.
- If list is empty TRUE is never set however there is no report to process
and it should (this bug). 2. When there are more than one report after process first it is set to TRUE however it should be after last.
But since all this appear in critical section i see no reason to have this handly made "last_report_read" semaphore in code at all. However I'm not familiar with wine code so my analyses may be wrong, but I hope it help someone fix this bug. This messages in console application are really annoying...
I'm not sure if you unterstood the code right - I did at first, too. There's no such thing as "there are more than one report". The while loop just feeds the readers, it's not delivering events down a queue, but up a subscription. Readers subscribe to events (that's the irp queue).
But maybe you are true that it should set TRUE outside the while loop - but only if anything was done. But since it's in a critical section, it doesn't really matter, I think. The outcome would be the same.
The driver itself doesn't implement a queue: It just tries to instantly deliver the report by calling process_hid_report(). If there's no reader queued yet, next time the driver sends a report, the current one will be lost. That's when you see the message.
OTOH, you probably don't want to implement a queue in the driver because otherwise your HID reports may drift in time a lot.
So the only proper way would be to accelerate the readers, that is deliver_last_report() which may have pretty high overhead as I was told.
I'm not sure what a HID report consists of: Is it single axes events and single button events? Or is it a structure of whole device state (axes position and button states)? In the latter case, an undelivered report could be coalesced into the one still sitting there, and maybe just degrade the error to a warning or remove it completely then.
I didn't have time to look into this completely but have some ideas.
Another idea of discussion: If the last report was not read yet: Should we overwrite the queued report or should be throw away the incoming report?