[PATCH] winebus.sys: Do not report HID report read errors unconditionally
Kai Krakow
kai at kaishome.de
Wed Aug 15 13:36:06 CDT 2018
Hello!
2018-08-15 19:49 GMT+02:00 Aric Stewart <aric at codeweavers.com>:
> On 8/15/18 1:47 AM, Kai Krakow wrote:
>> 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.
I've made a v2 patch and sent it to the list. I adjusted the commit
message to reflect the change.
I also tried to get completely rid of `last_report_read` with this but
it doesn't work because it is used in at least one place to track
delivery of the report just before working on the queues, it seems. I
wonder if this is necessary but this needs more investigation.
Aric, it in `dlls/winebus.sys/main.c`:
547 case IOCTL_HID_READ_REPORT:
548 {
549 TRACE_(hid_report)("IOCTL_HID_READ_REPORT\n");
550 EnterCriticalSection(&ext->report_cs);
551 status = ext->vtbl->begin_report_processing(device);
552 if (status != STATUS_SUCCESS)
553 {
554 irp->IoStatus.u.Status = status;
555 LeaveCriticalSection(&ext->report_cs);
556 break;
557 }
558 if (!ext->last_report_read)
^^^ Could this also be discarded?
559 {
560 irp->IoStatus.u.Status = status = deliver_last_report(ext,
561 irpsp->Parameters.DeviceIoControl.OutputBufferLength,
562 irp->UserBuffer, &irp->IoStatus.Information);
563 ext->last_report_read = TRUE;
564 }
565 else
566 {
567 InsertTailList(&ext->irp_queue,
&irp->Tail.Overlay.s.ListEntry);
568 status = STATUS_PENDING;
569 }
570 LeaveCriticalSection(&ext->report_cs);
571 break;
Regards,
Kai
More information about the wine-devel
mailing list