[PATCH v2 1/6] ntoskrnl.exe/tests: Silent a todo_wine failing test after a while.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- v2: * Fix spurious failures first. * Don't try NULL preparsed data, it crashes old windows versions. * Squash new HidP_Get*Caps tests together, add a few more with specific caps lookup. dlls/ntoskrnl.exe/tests/driver_hid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dlls/ntoskrnl.exe/tests/driver_hid.c b/dlls/ntoskrnl.exe/tests/driver_hid.c index 44ec1b09526..79b3e708752 100644 --- a/dlls/ntoskrnl.exe/tests/driver_hid.c +++ b/dlls/ntoskrnl.exe/tests/driver_hid.c @@ -103,6 +103,7 @@ static const unsigned char report_descriptor[] = static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) { + static int test_failed = 0; IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp); const ULONG in_size = stack->Parameters.DeviceIoControl.InputBufferLength; const ULONG out_size = stack->Parameters.DeviceIoControl.OutputBufferLength; @@ -176,7 +177,7 @@ static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) case IOCTL_HID_READ_REPORT: ok(!in_size, "got input size %u\n", in_size); - todo_wine ok(out_size == 2, "got output size %u\n", out_size); + if (test_failed++ < 10) todo_wine ok(out_size == 2, "got output size %u\n", out_size); ret = STATUS_NOT_IMPLEMENTED; break; -- 2.31.0
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 5e2520a3e12..769f37439a6 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1231,7 +1231,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(!got_bus_removal, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); @@ -1247,7 +1247,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(got_bus_removal == 1, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE); -- 2.31.0
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 5e2520a3e12..769f37439a6 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1231,7 +1231,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError());
pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(!got_bus_removal, "got %u bus removal messages\n", got_bus_removal);
set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); @@ -1247,7 +1247,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError());
pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(got_bus_removal == 1, "got %u bus removal messages\n", got_bus_removal);
set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE);
This seems less than ideal; does that machine really never send arrival messages?
On 6/3/21 5:41 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 5e2520a3e12..769f37439a6 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1231,7 +1231,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(!got_bus_removal, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); @@ -1247,7 +1247,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(got_bus_removal == 1, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE);
This seems less than ideal; does that machine really never send arrival messages?
It looks like so, at least it happens very often afaics, but on w7u_qxl only. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 6/3/21 10:43 AM, Rémi Bernon wrote:
On 6/3/21 5:41 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 5e2520a3e12..769f37439a6 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1231,7 +1231,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(!got_bus_removal, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); @@ -1247,7 +1247,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(got_bus_removal == 1, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE);
This seems less than ideal; does that machine really never send arrival messages?
It looks like so, at least it happens very often afaics, but on w7u_qxl only.
Actually, I'm not even seeing this failure anywhere on test.winehq.org; can you provide a link?
On 6/3/21 5:51 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 10:43 AM, Rémi Bernon wrote:
On 6/3/21 5:41 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 5e2520a3e12..769f37439a6 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1231,7 +1231,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(!got_bus_removal, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); @@ -1247,7 +1247,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(got_bus_removal == 1, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE);
This seems less than ideal; does that machine really never send arrival messages?
It looks like so, at least it happens very often afaics, but on w7u_qxl only.
Actually, I'm not even seeing this failure anywhere on test.winehq.org; can you provide a link?
It happened with the first version of the series, and almost every time I submit the patch manually on this machine: https://testbot.winehq.org/JobDetails.pl?Key=91705&f202=exe32.report#k202 -- Rémi Bernon <rbernon(a)codeweavers.com>
On 6/3/21 10:53 AM, Rémi Bernon wrote:
On 6/3/21 5:51 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 10:43 AM, Rémi Bernon wrote:
On 6/3/21 5:41 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 5e2520a3e12..769f37439a6 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1231,7 +1231,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(!got_bus_removal, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); @@ -1247,7 +1247,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(got_bus_removal == 1, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE);
This seems less than ideal; does that machine really never send arrival messages?
It looks like so, at least it happens very often afaics, but on w7u_qxl only.
Actually, I'm not even seeing this failure anywhere on test.winehq.org; can you provide a link?
It happened with the first version of the series, and almost every time I submit the patch manually on this machine:
https://testbot.winehq.org/JobDetails.pl?Key=91705&f202=exe32.report#k202
Sounds like there's a timing problem then; could we try increasing the timeout in pump_messages() instead?
On 6/3/21 6:04 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 10:53 AM, Rémi Bernon wrote:
On 6/3/21 5:51 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 10:43 AM, Rémi Bernon wrote:
On 6/3/21 5:41 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 5e2520a3e12..769f37439a6 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1231,7 +1231,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(!got_bus_removal, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); @@ -1247,7 +1247,7 @@ static void test_pnp_devices(void) ok(ret, "got error %u\n", GetLastError()); pump_messages(); - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", got_bus_arrival); + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus arrival messages\n", got_bus_arrival); ok(got_bus_removal == 1, "got %u bus removal messages\n", got_bus_removal); set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, DIGCF_DEVICEINTERFACE);
This seems less than ideal; does that machine really never send arrival messages?
It looks like so, at least it happens very often afaics, but on w7u_qxl only.
Actually, I'm not even seeing this failure anywhere on test.winehq.org; can you provide a link?
It happened with the first version of the series, and almost every time I submit the patch manually on this machine:
https://testbot.winehq.org/JobDetails.pl?Key=91705&f202=exe32.report#k202
Sounds like there's a timing problem then; could we try increasing the timeout in pump_messages() instead?
Increasing the timeout to 500ms and adding a while loop instead doesn't seem to help. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 6/3/21 2:31 PM, Rémi Bernon wrote:
On 6/3/21 6:04 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 10:53 AM, Rémi Bernon wrote:
On 6/3/21 5:51 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 10:43 AM, Rémi Bernon wrote:
On 6/3/21 5:41 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote: > Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> > --- > dlls/ntoskrnl.exe/tests/ntoskrnl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c > b/dlls/ntoskrnl.exe/tests/ntoskrnl.c > index 5e2520a3e12..769f37439a6 100644 > --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c > +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c > @@ -1231,7 +1231,7 @@ static void test_pnp_devices(void) > ok(ret, "got error %u\n", GetLastError()); > pump_messages(); > - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", > got_bus_arrival); > + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus > arrival messages\n", got_bus_arrival); > ok(!got_bus_removal, "got %u bus removal messages\n", > got_bus_removal); > set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, > DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); > @@ -1247,7 +1247,7 @@ static void test_pnp_devices(void) > ok(ret, "got error %u\n", GetLastError()); > pump_messages(); > - ok(got_bus_arrival == 1, "got %u bus arrival messages\n", > got_bus_arrival); > + ok(got_bus_arrival == 1 || broken(!got_bus_arrival), "got %u bus > arrival messages\n", got_bus_arrival); > ok(got_bus_removal == 1, "got %u bus removal messages\n", > got_bus_removal); > set = SetupDiGetClassDevsA(&bus_class, NULL, NULL, > DIGCF_DEVICEINTERFACE); >
This seems less than ideal; does that machine really never send arrival messages?
It looks like so, at least it happens very often afaics, but on w7u_qxl only.
Actually, I'm not even seeing this failure anywhere on test.winehq.org; can you provide a link?
It happened with the first version of the series, and almost every time I submit the patch manually on this machine:
https://testbot.winehq.org/JobDetails.pl?Key=91705&f202=exe32.report#k202
Sounds like there's a timing problem then; could we try increasing the timeout in pump_messages() instead?
Increasing the timeout to 500ms and adding a while loop instead doesn't seem to help.
I really don't like the idea of breaking the test completely like this. If nothing else I'd like to take some time to figure out what's wrong with that testbot machine.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/driver_hid.c | 28 ++++---- dlls/ntoskrnl.exe/tests/pop_hid_macros.h | 83 +++++++++++++++++++++++ dlls/ntoskrnl.exe/tests/psh_hid_macros.h | 85 ++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 12 deletions(-) create mode 100644 dlls/ntoskrnl.exe/tests/pop_hid_macros.h create mode 100644 dlls/ntoskrnl.exe/tests/psh_hid_macros.h diff --git a/dlls/ntoskrnl.exe/tests/driver_hid.c b/dlls/ntoskrnl.exe/tests/driver_hid.c index 79b3e708752..033c2925616 100644 --- a/dlls/ntoskrnl.exe/tests/driver_hid.c +++ b/dlls/ntoskrnl.exe/tests/driver_hid.c @@ -85,22 +85,26 @@ static NTSTATUS WINAPI driver_power(DEVICE_OBJECT *device, IRP *irp) return PoCallDriver(ext->NextDeviceObject, irp); } +#include "psh_hid_macros.h" + static const unsigned char report_descriptor[] = { - 0x05, HID_USAGE_PAGE_GENERIC, - 0x09, HID_USAGE_GENERIC_JOYSTICK, - 0xa1, 0x01, /* application collection */ - 0x05, HID_USAGE_PAGE_GENERIC, - 0x09, HID_USAGE_GENERIC_X, - 0x09, HID_USAGE_GENERIC_Y, - 0x15, 0x80, /* logical minimum -128 */ - 0x25, 0x7f, /* logical maximum 127 */ - 0x75, 0x08, /* report size */ - 0x95, 0x02, /* report count */ - 0x81, 0x02, /* input, variable */ - 0xc0, /* end collection */ + USAGE_PAGE(1, HID_USAGE_PAGE_GENERIC), + USAGE(1, HID_USAGE_GENERIC_JOYSTICK), + COLLECTION(1, Application), + USAGE_PAGE(1, HID_USAGE_PAGE_GENERIC), + USAGE(1, HID_USAGE_GENERIC_X), + USAGE(1, HID_USAGE_GENERIC_Y), + LOGICAL_MINIMUM(1, -128), + LOGICAL_MAXIMUM(1, 127), + REPORT_SIZE(1, 8), + REPORT_COUNT(1, 2), + INPUT(1, Data|Var|Abs), + END_COLLECTION, }; +#include "pop_hid_macros.h" + static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) { static int test_failed = 0; diff --git a/dlls/ntoskrnl.exe/tests/pop_hid_macros.h b/dlls/ntoskrnl.exe/tests/pop_hid_macros.h new file mode 100644 index 00000000000..767c26e8ecb --- /dev/null +++ b/dlls/ntoskrnl.exe/tests/pop_hid_macros.h @@ -0,0 +1,83 @@ +/* + * HID report helper macros. + * + * Copyright 2021 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#undef Data +#undef Cnst +#undef Array +#undef Var +#undef Abs +#undef Rel +#undef NoWrap +#undef Wrap +#undef NonLin +#undef Lin +#undef NoPref +#undef Pref +#undef NoNull +#undef Null +#undef NonVol +#undef Vol +#undef Bits +#undef Buff + +#undef Physical +#undef Application +#undef Logical +#undef Report +#undef NamedArray +#undef UsageSwitch +#undef UsageModifier + +#undef SHORT_ITEM_0 +#undef SHORT_ITEM_1 +#undef SHORT_ITEM_2 +#undef SHORT_ITEM_4 + +#undef LONG_ITEM + +#undef INPUT +#undef OUTPUT +#undef FEATURE +#undef COLLECTION +#undef END_COLLECTION + +#undef USAGE_PAGE +#undef LOGICAL_MINIMUM +#undef LOGICAL_MAXIMUM +#undef PHYSICAL_MINIMUM +#undef PHYSICAL_MAXIMUM +#undef UNIT_EXPONENT +#undef UNIT +#undef REPORT_SIZE +#undef REPORT_ID +#undef REPORT_COUNT +#undef PUSH +#undef POP + +#undef USAGE +#undef USAGE_MINIMUM +#undef USAGE_MAXIMUM +#undef DESIGNATOR_INDEX +#undef DESIGNATOR_MINIMUM +#undef DESIGNATOR_MAXIMUM +#undef STRING_INDEX +#undef STRING_MINIMUM +#undef STRING_MAXIMUM +#undef DELIMITER diff --git a/dlls/ntoskrnl.exe/tests/psh_hid_macros.h b/dlls/ntoskrnl.exe/tests/psh_hid_macros.h new file mode 100644 index 00000000000..4623af20598 --- /dev/null +++ b/dlls/ntoskrnl.exe/tests/psh_hid_macros.h @@ -0,0 +1,85 @@ +/* + * HID report helper macros. + * + * Copyright 2021 Rémi Bernon for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include <hidusage.h> + +#define Data 0 +#define Cnst 0x01 +#define Ary 0 +#define Var 0x02 +#define Abs 0 +#define Rel 0x04 +#define NoWrap 0 +#define Wrap 0x08 +#define NonLin 0 +#define Lin 0x10 +#define NoPref 0 +#define Pref 0x20 +#define NoNull 0 +#define Null 0x40 +#define NonVol 0 +#define Vol 0x80 +#define Bits 0 +#define Buff 0x100 + +#define Physical 0x00 +#define Application 0x01 +#define Logical 0x02 +#define Report 0x03 +#define NamedArray 0x04 +#define UsageSwitch 0x05 +#define UsageModifier 0x06 + +#define SHORT_ITEM_0(tag,type) (((tag)<<4)|((type)<<2)|0) +#define SHORT_ITEM_1(tag,type,data) (((tag)<<4)|((type)<<2)|1),((data)&0xff) +#define SHORT_ITEM_2(tag,type,data) (((tag)<<4)|((type)<<2)|2),((data)&0xff),(((data)>>8)&0xff) +#define SHORT_ITEM_4(tag,type,data) (((tag)<<4)|((type)<<2)|3),((data)&0xff),(((data)>>8)&0xff),(((data)>>16)&0xff),(((data)>>24)&0xff) + +#define LONG_ITEM(tag,size) SHORT_ITEM_2(0xf,0x3,((tag)<<8)|(size)) + +#define INPUT(n,data) SHORT_ITEM_##n(0x8,0,data) +#define OUTPUT(n,data) SHORT_ITEM_##n(0x9,0,data) +#define FEATURE(n,data) SHORT_ITEM_##n(0xb,0,data) +#define COLLECTION(n,data) SHORT_ITEM_##n(0xa,0,data) +#define END_COLLECTION SHORT_ITEM_0(0xc,0) + +#define USAGE_PAGE(n,data) SHORT_ITEM_##n(0x0,1,data) +#define LOGICAL_MINIMUM(n,data) SHORT_ITEM_##n(0x1,1,data) +#define LOGICAL_MAXIMUM(n,data) SHORT_ITEM_##n(0x2,1,data) +#define PHYSICAL_MINIMUM(n,data) SHORT_ITEM_##n(0x3,1,data) +#define PHYSICAL_MAXIMUM(n,data) SHORT_ITEM_##n(0x4,1,data) +#define UNIT_EXPONENT(n,data) SHORT_ITEM_##n(0x5,1,data) +#define UNIT(n,data) SHORT_ITEM_##n(0x6,1,data) +#define REPORT_SIZE(n,data) SHORT_ITEM_##n(0x7,1,data) +#define REPORT_ID(n,data) SHORT_ITEM_##n(0x8,1,data) +#define REPORT_COUNT(n,data) SHORT_ITEM_##n(0x9,1,data) +#define PUSH(n,data) SHORT_ITEM_##n(0xa,1,data) +#define POP(n,data) SHORT_ITEM_##n(0xb,1,data) + +#define USAGE(n,data) SHORT_ITEM_##n(0x0,2,data) +#define USAGE_MINIMUM(n,data) SHORT_ITEM_##n(0x1,2,data) +#define USAGE_MAXIMUM(n,data) SHORT_ITEM_##n(0x2,2,data) +#define DESIGNATOR_INDEX(n,data) SHORT_ITEM_##n(0x3,2,data) +#define DESIGNATOR_MINIMUM(n,data) SHORT_ITEM_##n(0x4,2,data) +#define DESIGNATOR_MAXIMUM(n,data) SHORT_ITEM_##n(0x5,2,data) +#define STRING_INDEX(n,data) SHORT_ITEM_##n(0x6,2,data) +#define STRING_MINIMUM(n,data) SHORT_ITEM_##n(0x7,2,data) +#define STRING_MAXIMUM(n,data) SHORT_ITEM_##n(0x8,2,data) +#define DELIMITER(n,data) SHORT_ITEM_##n(0x9,2,data) -- 2.31.0
I meant to offer this comment on the patches already committed, but maybe instead of having an extra file to #undef everything, we could just prepend HID_ to all of the definitions and leave them defined?
On 6/3/21 5:42 PM, Zebediah Figura (she/her) wrote:
I meant to offer this comment on the patches already committed, but maybe instead of having an extra file to #undef everything, we could just prepend HID_ to all of the definitions and leave them defined?
It's just making everything much more verbose, especially for INPUT / OUTPUT / FEATURE attributes (and IMHO much more horrible to read). Not having the prefixes also makes it much easier to reproduce template HID reports that are used in the specifications as example reports. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 6/3/21 10:46 AM, Rémi Bernon wrote:
On 6/3/21 5:42 PM, Zebediah Figura (she/her) wrote:
I meant to offer this comment on the patches already committed, but maybe instead of having an extra file to #undef everything, we could just prepend HID_ to all of the definitions and leave them defined?
It's just making everything much more verbose, especially for INPUT / OUTPUT / FEATURE attributes (and IMHO much more horrible to read).
Not having the prefixes also makes it much easier to reproduce template HID reports that are used in the specifications as example reports.
Okay, I guess I don't feel strongly about it.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/driver_hid.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/dlls/ntoskrnl.exe/tests/driver_hid.c b/dlls/ntoskrnl.exe/tests/driver_hid.c index 033c2925616..741fd9f639b 100644 --- a/dlls/ntoskrnl.exe/tests/driver_hid.c +++ b/dlls/ntoskrnl.exe/tests/driver_hid.c @@ -100,6 +100,27 @@ static const unsigned char report_descriptor[] = REPORT_SIZE(1, 8), REPORT_COUNT(1, 2), INPUT(1, Data|Var|Abs), + + USAGE_PAGE(1, HID_USAGE_PAGE_BUTTON), + USAGE_MINIMUM(1, 1), + USAGE_MAXIMUM(1, 8), + LOGICAL_MINIMUM(1, 0), + LOGICAL_MAXIMUM(1, 1), + PHYSICAL_MINIMUM(1, 0), + PHYSICAL_MAXIMUM(1, 1), + REPORT_COUNT(1, 8), + REPORT_SIZE(1, 1), + INPUT(1, Data|Var|Abs), + + USAGE_PAGE(1, HID_USAGE_PAGE_GENERIC), + USAGE(1, HID_USAGE_GENERIC_HATSWITCH), + LOGICAL_MINIMUM(1, 1), + LOGICAL_MAXIMUM(1, 8), + PHYSICAL_MINIMUM(1, 0), + PHYSICAL_MAXIMUM(1, 8), + REPORT_SIZE(1, 4), + REPORT_COUNT(1, 2), + INPUT(1, Data|Var|Abs), END_COLLECTION, }; @@ -181,7 +202,7 @@ static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) case IOCTL_HID_READ_REPORT: ok(!in_size, "got input size %u\n", in_size); - if (test_failed++ < 10) todo_wine ok(out_size == 2, "got output size %u\n", out_size); + if (test_failed++ < 10) todo_wine ok(out_size == 4, "got output size %u\n", out_size); ret = STATUS_NOT_IMPLEMENTED; break; -- 2.31.0
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/ntoskrnl.exe/tests/Makefile.in | 2 +- dlls/ntoskrnl.exe/tests/ntoskrnl.c | 248 ++++++++++++++++++++++++++++ 2 files changed, 249 insertions(+), 1 deletion(-) diff --git a/dlls/ntoskrnl.exe/tests/Makefile.in b/dlls/ntoskrnl.exe/tests/Makefile.in index 8c2115984c5..863fad30f63 100644 --- a/dlls/ntoskrnl.exe/tests/Makefile.in +++ b/dlls/ntoskrnl.exe/tests/Makefile.in @@ -1,5 +1,5 @@ TESTDLL = ntoskrnl.exe -IMPORTS = advapi32 crypt32 newdev setupapi user32 wintrust ws2_32 +IMPORTS = advapi32 crypt32 newdev setupapi user32 wintrust ws2_32 hid driver_IMPORTS = winecrt0 ntoskrnl driver_EXTRADLLFLAGS = -nodefaultlibs -nostartfiles -Wl,--subsystem,native diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 769f37439a6..4aa3284ad9c 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -40,6 +40,8 @@ #include "initguid.h" #include "devguid.h" #include "ddk/hidclass.h" +#include "ddk/hidsdi.h" +#include "ddk/hidpi.h" #include "wine/test.h" #include "wine/heap.h" #include "wine/mssign.h" @@ -1496,19 +1498,104 @@ static void test_pnp_driver(struct testsign_context *ctx) SetCurrentDirectoryA(cwd); } +#define check_button_caps(a, b) check_button_caps_(__LINE__, a, b) +static void check_button_caps_(int line, HIDP_BUTTON_CAPS *caps, HIDP_BUTTON_CAPS *exp) +{ + ok_(__FILE__, line)(caps->UsagePage == exp->UsagePage, "unexpected button caps UsagePage %x, expected %x\n", caps->UsagePage, exp->UsagePage); + ok_(__FILE__, line)(caps->ReportID == exp->ReportID, "unexpected button caps ReportID %d, expected %d\n", caps->ReportID, exp->ReportID); + ok_(__FILE__, line)(caps->IsAlias == exp->IsAlias, "unexpected button caps IsAlias %d, expected %d\n", caps->IsAlias, exp->IsAlias); + ok_(__FILE__, line)(caps->BitField == exp->BitField, "unexpected button caps BitField %d, expected %d\n", caps->BitField, exp->BitField); + ok_(__FILE__, line)(caps->LinkCollection == exp->LinkCollection, "unexpected button caps LinkCollection %d, expected %d\n", caps->LinkCollection, exp->LinkCollection); + ok_(__FILE__, line)(caps->LinkUsage == exp->LinkUsage, "unexpected button caps LinkUsage %x, expected %x\n", caps->LinkUsage, exp->LinkUsage); + ok_(__FILE__, line)(caps->LinkUsagePage == exp->LinkUsagePage, "unexpected button caps LinkUsagePage %x, expected %x\n", caps->LinkUsagePage, exp->LinkUsagePage); + ok_(__FILE__, line)(caps->IsRange == exp->IsRange, "unexpected button caps IsRange %d, expected %d\n", caps->IsRange, exp->IsRange); + ok_(__FILE__, line)(caps->IsStringRange == exp->IsStringRange, "unexpected button caps IsStringRange %d, expected %d\n", caps->IsStringRange, exp->IsStringRange); + ok_(__FILE__, line)(caps->IsDesignatorRange == exp->IsDesignatorRange, "unexpected button caps IsDesignatorRange %d, expected %d\n", caps->IsDesignatorRange, exp->IsDesignatorRange); + ok_(__FILE__, line)(caps->IsAbsolute == exp->IsAbsolute, "unexpected button caps IsAbsolute %d, expected %d\n", caps->IsAbsolute, exp->IsAbsolute); + + if (caps->IsRange && exp->IsRange) + { + ok_(__FILE__, line)(caps->Range.UsageMin == exp->Range.UsageMin, "unexpected button caps Range.UsageMin %d, expected %d\n", caps->Range.UsageMin, exp->Range.UsageMin); + ok_(__FILE__, line)(caps->Range.UsageMax == exp->Range.UsageMax, "unexpected button caps Range.UsageMax %d, expected %d\n", caps->Range.UsageMax, exp->Range.UsageMax); + ok_(__FILE__, line)(caps->Range.StringMin == exp->Range.StringMin, "unexpected button caps Range.StringMin %d, expected %d\n", caps->Range.StringMin, exp->Range.StringMin); + ok_(__FILE__, line)(caps->Range.StringMax == exp->Range.StringMax, "unexpected button caps Range.StringMax %d, expected %d\n", caps->Range.StringMax, exp->Range.StringMax); + ok_(__FILE__, line)(caps->Range.DesignatorMin == exp->Range.DesignatorMin, "unexpected button caps Range.DesignatorMin %d, expected %d\n", caps->Range.DesignatorMin, exp->Range.DesignatorMin); + ok_(__FILE__, line)(caps->Range.DesignatorMax == exp->Range.DesignatorMax, "unexpected button caps Range.DesignatorMax %d, expected %d\n", caps->Range.DesignatorMax, exp->Range.DesignatorMax); + ok_(__FILE__, line)(caps->Range.DataIndexMin == exp->Range.DataIndexMin, "unexpected button caps Range.DataIndexMin %d, expected %d\n", caps->Range.DataIndexMin, exp->Range.DataIndexMin); + ok_(__FILE__, line)(caps->Range.DataIndexMax == exp->Range.DataIndexMax, "unexpected button caps Range.DataIndexMax %d, expected %d\n", caps->Range.DataIndexMax, exp->Range.DataIndexMax); + } + else if (!caps->IsRange && !exp->IsRange) + { + ok_(__FILE__, line)(caps->NotRange.Usage == exp->NotRange.Usage, "unexpected button caps NotRange.Usage %d, expected %d\n", caps->NotRange.Usage, exp->NotRange.Usage); + ok_(__FILE__, line)(caps->NotRange.StringIndex == exp->NotRange.StringIndex, "unexpected button caps NotRange.StringIndex %d, expected %d\n", caps->NotRange.StringIndex, exp->NotRange.StringIndex); + ok_(__FILE__, line)(caps->NotRange.DesignatorIndex == exp->NotRange.DesignatorIndex, "unexpected button caps NotRange.DesignatorIndex %d, expected %d\n", caps->NotRange.DesignatorIndex, exp->NotRange.DesignatorIndex); + ok_(__FILE__, line)(caps->NotRange.DataIndex == exp->NotRange.DataIndex, "unexpected button caps NotRange.DataIndex %d, expected %d\n", caps->NotRange.DataIndex, exp->NotRange.DataIndex); + } +} + +#define check_value_caps(a, b) check_value_caps_(__LINE__, a, b) +static void check_value_caps_(int line, HIDP_VALUE_CAPS *caps, HIDP_VALUE_CAPS *exp) +{ + ok_(__FILE__, line)(caps->UsagePage == exp->UsagePage, "unexpected value caps UsagePage %x, expected %x\n", caps->UsagePage, exp->UsagePage); + ok_(__FILE__, line)(caps->ReportID == exp->ReportID, "unexpected value caps ReportID %d, expected %d\n", caps->ReportID, exp->ReportID); + ok_(__FILE__, line)(caps->IsAlias == exp->IsAlias, "unexpected value caps IsAlias %d, expected %d\n", caps->IsAlias, exp->IsAlias); + ok_(__FILE__, line)(caps->BitField == exp->BitField, "unexpected value caps BitField %d, expected %d\n", caps->BitField, exp->BitField); + ok_(__FILE__, line)(caps->LinkCollection == exp->LinkCollection, "unexpected value caps LinkCollection %d, expected %d\n", caps->LinkCollection, exp->LinkCollection); + ok_(__FILE__, line)(caps->LinkUsage == exp->LinkUsage, "unexpected value caps LinkUsage %x, expected %x\n", caps->LinkUsage, exp->LinkUsage); + ok_(__FILE__, line)(caps->LinkUsagePage == exp->LinkUsagePage, "unexpected value caps LinkUsagePage %x, expected %x\n", caps->LinkUsagePage, exp->LinkUsagePage); + ok_(__FILE__, line)(caps->IsRange == exp->IsRange, "unexpected value caps IsRange %d, expected %d\n", caps->IsRange, exp->IsRange); + ok_(__FILE__, line)(caps->IsStringRange == exp->IsStringRange, "unexpected value caps IsStringRange %d, expected %d\n", caps->IsStringRange, exp->IsStringRange); + ok_(__FILE__, line)(caps->IsDesignatorRange == exp->IsDesignatorRange, "unexpected value caps IsDesignatorRange %d, expected %d\n", caps->IsDesignatorRange, exp->IsDesignatorRange); + ok_(__FILE__, line)(caps->IsAbsolute == exp->IsAbsolute, "unexpected value caps IsAbsolute %d, expected %d\n", caps->IsAbsolute, exp->IsAbsolute); + + ok_(__FILE__, line)(caps->HasNull == exp->HasNull, "unexpected value caps HasNull %d, expected %d\n", caps->HasNull, exp->HasNull); + ok_(__FILE__, line)(caps->BitSize == exp->BitSize, "unexpected value caps BitSize %d, expected %d\n", caps->BitSize, exp->BitSize); + ok_(__FILE__, line)(caps->ReportCount == exp->ReportCount, "unexpected value caps ReportCount %d, expected %d\n", caps->ReportCount, exp->ReportCount); + ok_(__FILE__, line)(caps->UnitsExp == exp->UnitsExp, "unexpected value caps UnitsExp %d, expected %d\n", caps->UnitsExp, exp->UnitsExp); + ok_(__FILE__, line)(caps->Units == exp->Units, "unexpected value caps Units %d, expected %d\n", caps->Units, exp->Units); + ok_(__FILE__, line)(caps->LogicalMin == exp->LogicalMin, "unexpected value caps LogicalMin %d, expected %d\n", caps->LogicalMin, -exp->LogicalMin); + ok_(__FILE__, line)(caps->LogicalMax == exp->LogicalMax, "unexpected value caps LogicalMax %d, expected %d\n", caps->LogicalMax, exp->LogicalMax); + ok_(__FILE__, line)(caps->PhysicalMin == exp->PhysicalMin, "unexpected value caps PhysicalMin %d, expected %d\n", caps->PhysicalMin, exp->PhysicalMin); + ok_(__FILE__, line)(caps->PhysicalMax == exp->PhysicalMax, "unexpected value caps PhysicalMax %d, expected %d\n", caps->PhysicalMax, exp->PhysicalMax); + + if (caps->IsRange && exp->IsRange) + { + ok_(__FILE__, line)(caps->Range.UsageMin == exp->Range.UsageMin, "unexpected value caps Range.UsageMin %d, expected %d\n", caps->Range.UsageMin, exp->Range.UsageMin); + ok_(__FILE__, line)(caps->Range.UsageMax == exp->Range.UsageMax, "unexpected value caps Range.UsageMax %d, expected %d\n", caps->Range.UsageMax, exp->Range.UsageMax); + ok_(__FILE__, line)(caps->Range.StringMin == exp->Range.StringMin, "unexpected value caps Range.StringMin %d, expected %d\n", caps->Range.StringMin, exp->Range.StringMin); + ok_(__FILE__, line)(caps->Range.StringMax == exp->Range.StringMax, "unexpected value caps Range.StringMax %d, expected %d\n", caps->Range.StringMax, exp->Range.StringMax); + ok_(__FILE__, line)(caps->Range.DesignatorMin == exp->Range.DesignatorMin, "unexpected value caps Range.DesignatorMin %d, expected %d\n", caps->Range.DesignatorMin, exp->Range.DesignatorMin); + ok_(__FILE__, line)(caps->Range.DesignatorMax == exp->Range.DesignatorMax, "unexpected value caps Range.DesignatorMax %d, expected %d\n", caps->Range.DesignatorMax, exp->Range.DesignatorMax); + ok_(__FILE__, line)(caps->Range.DataIndexMin == exp->Range.DataIndexMin, "unexpected value caps Range.DataIndexMin %d, expected %d\n", caps->Range.DataIndexMin, exp->Range.DataIndexMin); + ok_(__FILE__, line)(caps->Range.DataIndexMax == exp->Range.DataIndexMax, "unexpected value caps Range.DataIndexMax %d, expected %d\n", caps->Range.DataIndexMax, exp->Range.DataIndexMax); + } + else if (!caps->IsRange && !exp->IsRange) + { + ok_(__FILE__, line)(caps->NotRange.Usage == exp->NotRange.Usage, "unexpected value caps NotRange.Usage %d, expected %d\n", caps->NotRange.Usage, exp->NotRange.Usage); + ok_(__FILE__, line)(caps->NotRange.StringIndex == exp->NotRange.StringIndex, "unexpected value caps NotRange.StringIndex %d, expected %d\n", caps->NotRange.StringIndex, exp->NotRange.StringIndex); + ok_(__FILE__, line)(caps->NotRange.DesignatorIndex == exp->NotRange.DesignatorIndex, "unexpected value caps NotRange.DesignatorIndex %d, expected %d\n", caps->NotRange.DesignatorIndex, exp->NotRange.DesignatorIndex); + ok_(__FILE__, line)(caps->NotRange.DataIndex == exp->NotRange.DataIndex, "unexpected value caps NotRange.DataIndex %d, expected %d\n", caps->NotRange.DataIndex, exp->NotRange.DataIndex); + } +} + static void test_hid_device(void) { char buffer[200]; SP_DEVICE_INTERFACE_DETAIL_DATA_A *iface_detail = (void *)buffer; SP_DEVICE_INTERFACE_DATA iface = {sizeof(iface)}; SP_DEVINFO_DATA device = {sizeof(device)}; + PHIDP_PREPARSED_DATA preparsed_data; + HIDP_BUTTON_CAPS button_caps[16]; + HIDP_VALUE_CAPS value_caps[16]; BOOL ret, found = FALSE; OBJECT_ATTRIBUTES attr; UNICODE_STRING string; IO_STATUS_BLOCK io; NTSTATUS status; + HIDP_CAPS caps; unsigned int i; HDEVINFO set; + USHORT count; HANDLE file; set = SetupDiGetClassDevsA(&GUID_DEVINTERFACE_HID, NULL, NULL, DIGCF_DEVICEINTERFACE | DIGCF_PRESENT); @@ -1541,6 +1628,167 @@ static void test_hid_device(void) FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL); ok(file != INVALID_HANDLE_VALUE, "got error %u\n", GetLastError()); + ret = HidD_GetPreparsedData(file, &preparsed_data); + ok(ret, "HidD_GetPreparsedData failed with error %u\n", GetLastError()); + + memset(buffer, 0, sizeof(buffer)); + status = HidP_GetCaps((PHIDP_PREPARSED_DATA)buffer, &caps); + ok(status == HIDP_STATUS_INVALID_PREPARSED_DATA, "HidP_GetCaps returned %#x\n", status); + status = HidP_GetCaps(preparsed_data, &caps); + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetCaps returned %#x\n", status); + + ok(caps.Usage == HID_USAGE_GENERIC_JOYSTICK, "unexpected caps Usage %x, expected %x\n", caps.Usage, HID_USAGE_GENERIC_JOYSTICK); + ok(caps.UsagePage == HID_USAGE_PAGE_GENERIC, "unexpected caps UsagePage %x, expected %x\n", caps.UsagePage, HID_USAGE_PAGE_GENERIC); + ok(caps.InputReportByteLength == 5, "unexpected caps InputReportByteLength %d, expected %d\n", caps.InputReportByteLength, 5); + ok(caps.OutputReportByteLength == 0, "unexpected caps OutputReportByteLength %d, expected %d\n", caps.OutputReportByteLength, 0); + ok(caps.FeatureReportByteLength == 0, "unexpected caps FeatureReportByteLength %d, expected %d\n", caps.FeatureReportByteLength, 0); + ok(caps.NumberLinkCollectionNodes == 1, "unexpected caps NumberLinkCollectionNodes %d, expected %d\n", caps.NumberLinkCollectionNodes, 1); + ok(caps.NumberInputButtonCaps == 1, "unexpected caps NumberInputButtonCaps %d, expected %d\n", caps.NumberInputButtonCaps, 1); + ok(caps.NumberInputValueCaps == 3, "unexpected caps NumberInputValueCaps %d, expected %d\n", caps.NumberInputValueCaps, 3); + ok(caps.NumberInputDataIndices == 11, "unexpected caps NumberInputDataIndices %d, expected %d\n", caps.NumberInputDataIndices, 11); + ok(caps.NumberOutputButtonCaps == 0, "unexpected caps NumberOutputButtonCaps %d, expected %d\n", caps.NumberOutputButtonCaps, 0); + ok(caps.NumberOutputValueCaps == 0, "unexpected caps NumberOutputValueCaps %d, expected %d\n", caps.NumberOutputValueCaps, 0); + ok(caps.NumberOutputDataIndices == 0, "unexpected caps NumberOutputDataIndices %d, expected %d\n", caps.NumberOutputDataIndices, 0); + ok(caps.NumberFeatureButtonCaps == 0, "unexpected caps NumberFeatureButtonCaps %d, expected %d\n", caps.NumberFeatureButtonCaps, 0); + ok(caps.NumberFeatureValueCaps == 0, "unexpected caps NumberFeatureValueCaps %d, expected %d\n", caps.NumberFeatureValueCaps, 0); + ok(caps.NumberFeatureDataIndices == 0, "unexpected caps NumberFeatureDataIndices %d, expected %d\n", caps.NumberFeatureDataIndices, 0); + + count = 0; + status = HidP_GetButtonCaps(HidP_Output, button_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetButtonCaps returned %#x\n", status); + status = HidP_GetButtonCaps(HidP_Feature + 1, button_caps, &count, preparsed_data); + ok(status == HIDP_STATUS_INVALID_REPORT_TYPE, "HidP_GetButtonCaps returned %#x\n", status); + status = HidP_GetButtonCaps(HidP_Input, button_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetButtonCaps returned %#x\n", status); + count = ARRAY_SIZE(button_caps); + status = HidP_GetButtonCaps(HidP_Input, button_caps, &count, (PHIDP_PREPARSED_DATA)buffer); + ok(status == HIDP_STATUS_INVALID_PREPARSED_DATA, "HidP_GetButtonCaps returned %#x\n", status); + status = HidP_GetButtonCaps(HidP_Input, button_caps, &count, preparsed_data); + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetButtonCaps returned %#x\n", status); + ok(count == caps.NumberInputButtonCaps, "HidP_GetButtonCaps returned count %d, expected %d\n", count, caps.NumberInputButtonCaps); + + ok(button_caps[0].UsagePage == HID_USAGE_PAGE_BUTTON, "unexpected button caps UsagePage %x, expected %x\n", button_caps[0].UsagePage, HID_USAGE_PAGE_BUTTON); + ok(button_caps[0].ReportID == 0, "unexpected button caps ReportID %d, expected %d\n", button_caps[0].ReportID, 0); + ok(!button_caps[0].IsAlias, "unexpected button caps IsAlias %d, expected %d\n", button_caps[0].IsAlias, 0); + todo_wine ok(button_caps[0].BitField == 2, "unexpected button caps BitField %d, expected %d\n", button_caps[0].BitField, 2); + ok(button_caps[0].LinkCollection == 0, "unexpected button caps LinkCollection %d, expected %d\n", button_caps[0].LinkCollection, 0); + ok(button_caps[0].LinkUsage == HID_USAGE_GENERIC_JOYSTICK, "unexpected button caps LinkUsage %x, expected %x\n", button_caps[0].LinkUsage, HID_USAGE_GENERIC_JOYSTICK); + ok(button_caps[0].LinkUsagePage == HID_USAGE_PAGE_GENERIC, "unexpected button caps LinkUsagePage %x, expected %x\n", button_caps[0].LinkUsagePage, HID_USAGE_PAGE_GENERIC); + ok(button_caps[0].IsRange, "unexpected button caps IsRange %d, expected %d\n", button_caps[0].IsRange, 1); + ok(!button_caps[0].IsStringRange, "unexpected button caps IsStringRange %d, expected %d\n", button_caps[0].IsStringRange, 0); + ok(!button_caps[0].IsDesignatorRange, "unexpected button caps IsDesignatorRange %d, expected %d\n", button_caps[0].IsDesignatorRange, 0); + ok(button_caps[0].IsAbsolute, "unexpected button caps IsAbsolute %d, expected %d\n", button_caps[0].IsAbsolute, 1); + ok(button_caps[0].Range.UsageMin == 1, "unexpected button caps Range.UsageMin %d, expected %d\n", button_caps[0].Range.UsageMin, 1); + ok(button_caps[0].Range.UsageMax == 8, "unexpected button caps Range.UsageMax %d, expected %d\n", button_caps[0].Range.UsageMax, 8); + ok(button_caps[0].Range.StringMin == 0, "unexpected button caps Range.StringMin %d, expected %d\n", button_caps[0].Range.StringMin, 0); + ok(button_caps[0].Range.StringMax == 0, "unexpected button caps Range.StringMax %d, expected %d\n", button_caps[0].Range.StringMax, 0); + ok(button_caps[0].Range.DesignatorMin == 0, "unexpected button caps Range.DesignatorMin %d, expected %d\n", button_caps[0].Range.DesignatorMin, 0); + ok(button_caps[0].Range.DesignatorMax == 0, "unexpected button caps Range.DesignatorMax %d, expected %d\n", button_caps[0].Range.DesignatorMax, 0); + ok(button_caps[0].Range.DataIndexMin == 2, "unexpected button caps Range.DataIndexMin %d, expected %d\n", button_caps[0].Range.DataIndexMin, 2); + ok(button_caps[0].Range.DataIndexMax == 9, "unexpected button caps Range.DataIndexMax %d, expected %d\n", button_caps[0].Range.DataIndexMax, 9); + + count = 0; + status = HidP_GetSpecificButtonCaps(HidP_Output, 0, 0, 0, button_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetSpecificButtonCaps returned %#x\n", status); + status = HidP_GetSpecificButtonCaps(HidP_Feature + 1, 0, 0, 0, button_caps, &count, preparsed_data); + ok(status == HIDP_STATUS_INVALID_REPORT_TYPE, "HidP_GetSpecificButtonCaps returned %#x\n", status); + status = HidP_GetSpecificButtonCaps(HidP_Input, 0, 0, 0, button_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetSpecificButtonCaps returned %#x\n", status); + count = ARRAY_SIZE(button_caps) - 1; + status = HidP_GetSpecificButtonCaps(HidP_Input, 0, 0, 0, button_caps, &count, (PHIDP_PREPARSED_DATA)buffer); + ok(status == HIDP_STATUS_INVALID_PREPARSED_DATA, "HidP_GetSpecificButtonCaps returned %#x\n", status); + + status = HidP_GetSpecificButtonCaps(HidP_Input, 0, 0, 0, button_caps + 1, &count, preparsed_data); + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetSpecificButtonCaps returned %#x\n", status); + ok(count == caps.NumberInputButtonCaps, "HidP_GetSpecificButtonCaps returned count %d, expected %d\n", count, caps.NumberInputButtonCaps); + check_button_caps(&button_caps[1], &button_caps[0]); + + status = HidP_GetSpecificButtonCaps(HidP_Input, HID_USAGE_PAGE_BUTTON, 0, 5, button_caps + 1, &count, preparsed_data); + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetSpecificButtonCaps returned %#x\n", status); + ok(count == 1, "HidP_GetSpecificButtonCaps returned count %d, expected %d\n", count, 1); + check_button_caps(&button_caps[1], &button_caps[0]); + + status = HidP_GetSpecificButtonCaps(HidP_Input, 0xfffe, 0, 0, button_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetSpecificButtonCaps returned %#x\n", status); + status = HidP_GetSpecificButtonCaps(HidP_Input, 0, 0xfffe, 0, button_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetSpecificButtonCaps returned %#x\n", status); + status = HidP_GetSpecificButtonCaps(HidP_Input, 0, 0, 0xfffe, button_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetSpecificButtonCaps returned %#x\n", status); + + count = 0; + status = HidP_GetValueCaps(HidP_Output, value_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetValueCaps returned %#x\n", status); + status = HidP_GetValueCaps(HidP_Feature + 1, value_caps, &count, preparsed_data); + ok(status == HIDP_STATUS_INVALID_REPORT_TYPE, "HidP_GetValueCaps returned %#x\n", status); + status = HidP_GetValueCaps(HidP_Input, value_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetValueCaps returned %#x\n", status); + count = ARRAY_SIZE(value_caps); + status = HidP_GetValueCaps(HidP_Input, value_caps, &count, (PHIDP_PREPARSED_DATA)buffer); + ok(status == HIDP_STATUS_INVALID_PREPARSED_DATA, "HidP_GetValueCaps returned %#x\n", status); + status = HidP_GetValueCaps(HidP_Input, value_caps, &count, preparsed_data); + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetValueCaps returned %#x\n", status); + ok(count == caps.NumberInputValueCaps, "HidP_GetValueCaps returned count %d, expected %d\n", count, caps.NumberInputButtonCaps); + + ok(value_caps[0].UsagePage == HID_USAGE_PAGE_GENERIC, "unexpected value caps UsagePage %x, expected %x\n", value_caps[0].UsagePage, HID_USAGE_PAGE_GENERIC); + ok(value_caps[0].ReportID == 0, "unexpected value caps ReportID %d, expected %d\n", value_caps[0].ReportID, 0); + ok(value_caps[0].IsAlias == 0, "unexpected value caps IsAlias %d, expected %d\n", value_caps[0].IsAlias, 0); + todo_wine ok(value_caps[0].BitField == 2, "unexpected value caps BitField %d, expected %d\n", value_caps[0].BitField, 2); + ok(value_caps[0].LinkCollection == 0, "unexpected value caps LinkCollection %d, expected %d\n", value_caps[0].LinkCollection, 0); + ok(value_caps[0].LinkUsage == HID_USAGE_GENERIC_JOYSTICK, "unexpected value caps LinkUsage %x, expected %x\n", value_caps[0].LinkUsage, HID_USAGE_GENERIC_JOYSTICK); + ok(value_caps[0].LinkUsagePage == HID_USAGE_PAGE_GENERIC, "unexpected value caps LinkUsagePage %x, expected %x\n", value_caps[0].LinkUsagePage, HID_USAGE_PAGE_GENERIC); + ok(!value_caps[0].IsRange, "unexpected value caps IsRange %d, expected %d\n", value_caps[0].IsRange, 0); + ok(!value_caps[0].IsStringRange, "unexpected value caps IsStringRange %d, expected %d\n", value_caps[0].IsStringRange, 0); + ok(!value_caps[0].IsDesignatorRange, "unexpected value caps IsDesignatorRange %d, expected %d\n", value_caps[0].IsDesignatorRange, 0); + ok(value_caps[0].IsAbsolute, "unexpected value caps IsAbsolute %d, expected %d\n", value_caps[0].IsAbsolute, 1); + ok(value_caps[0].HasNull == 0, "unexpected value caps HasNull %d, expected %d\n", value_caps[0].HasNull, 0); + ok(value_caps[0].BitSize == 8, "unexpected value caps BitSize %d, expected %d\n", value_caps[0].BitSize, 8); + ok(value_caps[0].ReportCount == 1, "unexpected value caps ReportCount %d, expected %d\n", value_caps[0].ReportCount, 1); + ok(value_caps[0].UnitsExp == 0, "unexpected value caps UnitsExp %d, expected %d\n", value_caps[0].UnitsExp, 0); + ok(value_caps[0].Units == 0, "unexpected value caps Units %d, expected %d\n", value_caps[0].Units, 0); + ok(value_caps[0].LogicalMin == -128, "unexpected value caps LogicalMin %d, expected %d\n", value_caps[0].LogicalMin, -128); + ok(value_caps[0].LogicalMax == 127, "unexpected value caps LogicalMax %d, expected %d\n", value_caps[0].LogicalMax, 127); + ok(value_caps[0].PhysicalMin == 0, "unexpected value caps PhysicalMin %d, expected %d\n", value_caps[0].PhysicalMin, 0); + ok(value_caps[0].PhysicalMax == 0, "unexpected value caps PhysicalMax %d, expected %d\n", value_caps[0].PhysicalMax, 0); + todo_wine ok(value_caps[0].NotRange.Usage == HID_USAGE_GENERIC_Y, "unexpected value caps NotRange.Usage %d, expected %d\n", value_caps[0].NotRange.Usage, HID_USAGE_GENERIC_Y); + ok(value_caps[0].NotRange.StringIndex == 0, "unexpected value caps NotRange.StringIndex %d, expected %d\n", value_caps[0].NotRange.StringIndex, 0); + ok(value_caps[0].NotRange.DesignatorIndex == 0, "unexpected value caps NotRange.DesignatorIndex %d, expected %d\n", value_caps[0].NotRange.DesignatorIndex, 0); + ok(value_caps[0].NotRange.DataIndex == 0, "unexpected value caps NotRange.DataIndex %d, expected %d\n", value_caps[0].NotRange.DataIndex, 0); + + todo_wine ok(value_caps[1].NotRange.Usage == HID_USAGE_GENERIC_X, "unexpected value caps NotRange.Usage %d, expected %d\n", value_caps[0].NotRange.Usage, HID_USAGE_GENERIC_X); + ok(value_caps[2].NotRange.Usage == HID_USAGE_GENERIC_HATSWITCH, "unexpected value caps NotRange.Usage %d, expected %d\n", value_caps[0].NotRange.Usage, HID_USAGE_GENERIC_HATSWITCH); + + count = 0; + status = HidP_GetSpecificValueCaps(HidP_Output, 0, 0, 0, value_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetSpecificValueCaps returned %#x\n", status); + status = HidP_GetSpecificValueCaps(HidP_Feature + 1, 0, 0, 0, value_caps, &count, preparsed_data); + ok(status == HIDP_STATUS_INVALID_REPORT_TYPE, "HidP_GetSpecificValueCaps returned %#x\n", status); + status = HidP_GetSpecificValueCaps(HidP_Input, 0, 0, 0, value_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetSpecificValueCaps returned %#x\n", status); + count = ARRAY_SIZE(value_caps) - 3; + status = HidP_GetSpecificValueCaps(HidP_Input, 0, 0, 0, value_caps + 3, &count, (PHIDP_PREPARSED_DATA)buffer); + ok(status == HIDP_STATUS_INVALID_PREPARSED_DATA, "HidP_GetSpecificValueCaps returned %#x\n", status); + + status = HidP_GetSpecificValueCaps(HidP_Input, 0, 0, 0, value_caps + 3, &count, preparsed_data); + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetSpecificValueCaps returned %#x\n", status); + ok(count == caps.NumberInputValueCaps, "HidP_GetSpecificValueCaps returned count %d, expected %d\n", count, caps.NumberInputButtonCaps); + check_value_caps(&value_caps[3], &value_caps[0]); + check_value_caps(&value_caps[4], &value_caps[1]); + check_value_caps(&value_caps[5], &value_caps[2]); + + count = ARRAY_SIZE(value_caps) - 3; + status = HidP_GetSpecificValueCaps(HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_HATSWITCH, value_caps + 3, &count, preparsed_data); + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetSpecificValueCaps returned %#x\n", status); + ok(count == 1, "HidP_GetSpecificValueCaps returned count %d, expected %d\n", count, 1); + check_value_caps(&value_caps[3], &value_caps[2]); + + status = HidP_GetSpecificValueCaps(HidP_Input, 0xfffe, 0, 0, value_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetSpecificValueCaps returned %#x\n", status); + status = HidP_GetSpecificValueCaps(HidP_Input, 0, 0xfffe, 0, value_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetSpecificValueCaps returned %#x\n", status); + status = HidP_GetSpecificValueCaps(HidP_Input, 0, 0, 0xfffe, value_caps, &count, preparsed_data); + todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetSpecificValueCaps returned %#x\n", status); + + HidD_FreePreparsedData(preparsed_data); CloseHandle(file); RtlInitUnicodeString(&string, L"\\??\\root#winetest#0#{deadbeef-29ef-4538-a5fd-b69573a362c0}"); -- 2.31.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=91742 Your paranoid android. === w864 (64 bit report) === Report validation errors: ntoskrnl.exe:ntoskrnl crashed (c0000374)
On 6/3/21 3:43 PM, Marvin wrote:
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=91742
Your paranoid android.
=== w864 (64 bit report) ===
Report validation errors: ntoskrnl.exe:ntoskrnl crashed (c0000374)
I think it's not even supposed to run on w864, as it says ntoskrnl.c:349: Tests skipped: Failed to start service; probably your machine doesn't accept unsigned drivers. But that crash seems genuine nonetheless, although I don't really understand how it happens... I'll have a look. PATCH < 5 should be fine I guess. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 6/3/21 9:37 AM, Rémi Bernon wrote:
On 6/3/21 3:43 PM, Marvin wrote:
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=91742
Your paranoid android.
=== w864 (64 bit report) ===
Report validation errors: ntoskrnl.exe:ntoskrnl crashed (c0000374)
I think it's not even supposed to run on w864, as it says
ntoskrnl.c:349: Tests skipped: Failed to start service; probably your machine doesn't accept unsigned drivers.
But that crash seems genuine nonetheless, although I don't really understand how it happens... I'll have a look.
PATCH < 5 should be fine I guess.
FWIW, that's STATUS_HEAP_CORRUPTION.
On 6/3/21 5:43 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 9:37 AM, Rémi Bernon wrote:
On 6/3/21 3:43 PM, Marvin wrote:
Hi,
While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=91742
Your paranoid android.
=== w864 (64 bit report) ===
Report validation errors: ntoskrnl.exe:ntoskrnl crashed (c0000374)
I think it's not even supposed to run on w864, as it says
ntoskrnl.c:349: Tests skipped: Failed to start service; probably your machine doesn't accept unsigned drivers.
But that crash seems genuine nonetheless, although I don't really understand how it happens... I'll have a look.
PATCH < 5 should be fine I guess.
FWIW, that's STATUS_HEAP_CORRUPTION.
Okay, and it seems to be crashing on: ret = CertCloseStore(ctx->root_store, CERT_CLOSE_STORE_CHECK_FLAG); in testsign_cleanup. And it doesn't crash anymore if I add a ok(0, "...\n"); line before it. I think there's some corruption lurking around, and the crash has nothing to do with the patch as the code it adds isn't even executed. -- Rémi Bernon <rbernon(a)codeweavers.com>
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/hidp.c | 3 +++ dlls/ntoskrnl.exe/tests/ntoskrnl.c | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c index c4d162695a9..fd2ac86da32 100644 --- a/dlls/hid/hidp.c +++ b/dlls/hid/hidp.c @@ -1002,6 +1002,9 @@ NTSTATUS WINAPI HidP_GetLinkCollectionNodes(HIDP_LINK_COLLECTION_NODE *LinkColle TRACE("(%p, %p, %p)\n", LinkCollectionNode, LinkCollectionNodeLength, PreparsedData); + if (data->magic != HID_MAGIC) + return HIDP_STATUS_INVALID_PREPARSED_DATA; + if (*LinkCollectionNodeLength < data->caps.NumberLinkCollectionNodes) return HIDP_STATUS_BUFFER_TOO_SMALL; diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c index 4aa3284ad9c..cfd307aa52e 100644 --- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c +++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c @@ -1584,10 +1584,12 @@ static void test_hid_device(void) SP_DEVICE_INTERFACE_DETAIL_DATA_A *iface_detail = (void *)buffer; SP_DEVICE_INTERFACE_DATA iface = {sizeof(iface)}; SP_DEVINFO_DATA device = {sizeof(device)}; + HIDP_LINK_COLLECTION_NODE collections[16]; PHIDP_PREPARSED_DATA preparsed_data; HIDP_BUTTON_CAPS button_caps[16]; HIDP_VALUE_CAPS value_caps[16]; BOOL ret, found = FALSE; + DWORD collection_count; OBJECT_ATTRIBUTES attr; UNICODE_STRING string; IO_STATUS_BLOCK io; @@ -1653,6 +1655,25 @@ static void test_hid_device(void) ok(caps.NumberFeatureValueCaps == 0, "unexpected caps NumberFeatureValueCaps %d, expected %d\n", caps.NumberFeatureValueCaps, 0); ok(caps.NumberFeatureDataIndices == 0, "unexpected caps NumberFeatureDataIndices %d, expected %d\n", caps.NumberFeatureDataIndices, 0); + collection_count = 0; + status = HidP_GetLinkCollectionNodes(collections, &collection_count, preparsed_data); + ok(status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetLinkCollectionNodes returned %#x\n", status); + collection_count = ARRAY_SIZE(collections); + status = HidP_GetLinkCollectionNodes(collections, &collection_count, (PHIDP_PREPARSED_DATA)buffer); + ok(status == HIDP_STATUS_INVALID_PREPARSED_DATA, "HidP_GetLinkCollectionNodes returned %#x\n", status); + status = HidP_GetLinkCollectionNodes(collections, &collection_count, preparsed_data); + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetLinkCollectionNodes returned %#x\n", status); + ok(collection_count == caps.NumberLinkCollectionNodes, "HidP_GetLinkCollectionNodes returned count %d, expected %d\n", collection_count, caps.NumberLinkCollectionNodes); + + ok(collections[0].LinkUsage == HID_USAGE_GENERIC_JOYSTICK, "unexpected collection LinkUsage %x, expected %x\n", collections[0].LinkUsage, HID_USAGE_GENERIC_JOYSTICK); + ok(collections[0].LinkUsagePage == HID_USAGE_PAGE_GENERIC, "unexpected collection LinkUsagePage %x, expected %x\n", collections[0].LinkUsagePage, HID_USAGE_PAGE_GENERIC); + ok(collections[0].Parent == 0, "unexpected collection Parent %d, expected %d\n", collections[0].Parent, 0); + ok(collections[0].NumberOfChildren == 0, "unexpected collection NumberOfChildren %d, expected %d\n", collections[0].NumberOfChildren, 0); + ok(collections[0].NextSibling == 0, "unexpected collection NextSibling %d, expected %d\n", collections[0].NextSibling, 0); + ok(collections[0].FirstChild == 0, "unexpected collection FirstChild %d, expected %d\n", collections[0].FirstChild, 0); + ok(collections[0].CollectionType == 1, "unexpected collection CollectionType %d, expected %d\n", collections[0].CollectionType, 1); + ok(collections[0].IsAlias == 0, "unexpected collection IsAlias %d, expected %d\n", collections[0].IsAlias, 0); + count = 0; status = HidP_GetButtonCaps(HidP_Output, button_caps, &count, preparsed_data); todo_wine ok(status == HIDP_STATUS_USAGE_NOT_FOUND, "HidP_GetButtonCaps returned %#x\n", status); -- 2.31.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=91743 Your paranoid android. === w7u_el (32 bit report) === ntoskrnl.exe: driver.c:759: Test failed: got 0 === w8 (32 bit report) === ntoskrnl.exe: driver.c:759: Test failed: got 0 === w864 (64 bit report) === Report validation errors: ntoskrnl.exe:ntoskrnl crashed (c0000374)
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
v2: * Fix spurious failures first.
* Don't try NULL preparsed data, it crashes old windows versions.
* Squash new HidP_Get*Caps tests together, add a few more with specific caps lookup.
dlls/ntoskrnl.exe/tests/driver_hid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/tests/driver_hid.c b/dlls/ntoskrnl.exe/tests/driver_hid.c index 44ec1b09526..79b3e708752 100644 --- a/dlls/ntoskrnl.exe/tests/driver_hid.c +++ b/dlls/ntoskrnl.exe/tests/driver_hid.c @@ -103,6 +103,7 @@ static const unsigned char report_descriptor[] =
static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) { + static int test_failed = 0; IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp); const ULONG in_size = stack->Parameters.DeviceIoControl.InputBufferLength; const ULONG out_size = stack->Parameters.DeviceIoControl.OutputBufferLength; @@ -176,7 +177,7 @@ static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp)
case IOCTL_HID_READ_REPORT: ok(!in_size, "got input size %u\n", in_size); - todo_wine ok(out_size == 2, "got output size %u\n", out_size); + if (test_failed++ < 10) todo_wine ok(out_size == 2, "got output size %u\n", out_size);
ret = STATUS_NOT_IMPLEMENTED; break;
Does this test fail on Windows? Can you provide a link to the test results?
On 6/3/21 5:40 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
v2: * Fix spurious failures first.
* Don't try NULL preparsed data, it crashes old windows versions.
* Squash new HidP_Get*Caps tests together, add a few more with specific caps lookup.
dlls/ntoskrnl.exe/tests/driver_hid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/tests/driver_hid.c b/dlls/ntoskrnl.exe/tests/driver_hid.c index 44ec1b09526..79b3e708752 100644 --- a/dlls/ntoskrnl.exe/tests/driver_hid.c +++ b/dlls/ntoskrnl.exe/tests/driver_hid.c @@ -103,6 +103,7 @@ static const unsigned char report_descriptor[] = static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) { + static int test_failed = 0; IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp); const ULONG in_size = stack->Parameters.DeviceIoControl.InputBufferLength; const ULONG out_size = stack->Parameters.DeviceIoControl.OutputBufferLength; @@ -176,7 +177,7 @@ static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) case IOCTL_HID_READ_REPORT: ok(!in_size, "got input size %u\n", in_size); - todo_wine ok(out_size == 2, "got output size %u\n", out_size); + if (test_failed++ < 10) todo_wine ok(out_size == 2, "got output size %u\n", out_size); ret = STATUS_NOT_IMPLEMENTED; break;
Does this test fail on Windows? Can you provide a link to the test results?
It's not failing on windows, but the todo_wine are spamming the test results, making the test be reported as failing because too much data is printed. -- Rémi Bernon <rbernon(a)codeweavers.com>
On 6/3/21 10:42 AM, Rémi Bernon wrote:
On 6/3/21 5:40 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
v2: * Fix spurious failures first.
* Don't try NULL preparsed data, it crashes old windows versions.
* Squash new HidP_Get*Caps tests together, add a few more with specific caps lookup.
dlls/ntoskrnl.exe/tests/driver_hid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/tests/driver_hid.c b/dlls/ntoskrnl.exe/tests/driver_hid.c index 44ec1b09526..79b3e708752 100644 --- a/dlls/ntoskrnl.exe/tests/driver_hid.c +++ b/dlls/ntoskrnl.exe/tests/driver_hid.c @@ -103,6 +103,7 @@ static const unsigned char report_descriptor[] = static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) { + static int test_failed = 0; IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp); const ULONG in_size = stack->Parameters.DeviceIoControl.InputBufferLength; const ULONG out_size = stack->Parameters.DeviceIoControl.OutputBufferLength; @@ -176,7 +177,7 @@ static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) case IOCTL_HID_READ_REPORT: ok(!in_size, "got input size %u\n", in_size); - todo_wine ok(out_size == 2, "got output size %u\n", out_size); + if (test_failed++ < 10) todo_wine ok(out_size == 2, "got output size %u\n", out_size); ret = STATUS_NOT_IMPLEMENTED; break;
Does this test fail on Windows? Can you provide a link to the test results?
It's not failing on windows, but the todo_wine are spamming the test results, making the test be reported as failing because too much data is printed.
Shame on me for introducing that test and never fixing it, but maybe we could just fix it the right way instead?
On 6/3/21 5:44 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 10:42 AM, Rémi Bernon wrote:
On 6/3/21 5:40 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
v2: * Fix spurious failures first.
* Don't try NULL preparsed data, it crashes old windows versions.
* Squash new HidP_Get*Caps tests together, add a few more with specific caps lookup.
dlls/ntoskrnl.exe/tests/driver_hid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/tests/driver_hid.c b/dlls/ntoskrnl.exe/tests/driver_hid.c index 44ec1b09526..79b3e708752 100644 --- a/dlls/ntoskrnl.exe/tests/driver_hid.c +++ b/dlls/ntoskrnl.exe/tests/driver_hid.c @@ -103,6 +103,7 @@ static const unsigned char report_descriptor[] = static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) { + static int test_failed = 0; IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp); const ULONG in_size = stack->Parameters.DeviceIoControl.InputBufferLength; const ULONG out_size = stack->Parameters.DeviceIoControl.OutputBufferLength; @@ -176,7 +177,7 @@ static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) case IOCTL_HID_READ_REPORT: ok(!in_size, "got input size %u\n", in_size); - todo_wine ok(out_size == 2, "got output size %u\n", out_size); + if (test_failed++ < 10) todo_wine ok(out_size == 2, "got output size %u\n", out_size); ret = STATUS_NOT_IMPLEMENTED; break;
Does this test fail on Windows? Can you provide a link to the test results?
It's not failing on windows, but the todo_wine are spamming the test results, making the test be reported as failing because too much data is printed.
Shame on me for introducing that test and never fixing it, but maybe we could just fix it the right way instead?
Yes but it's a little bit more complicated than it seems, and I'm a bit afraid of the possible unforeseen consequences. There's apparently several ways to request reports (in polled and non-polled modes for instance). and I couldn't test the IOCTL_HID_GET_INPUT_REPORT way yet for instance, I don't know if it passes a packet with the full report length, including 0 report id or if it uses a different length depending on when report ids are used or not. Also in winebus.sys, both ways are handled by deliver_last_report, which expects a full report. Then there's the question whether reading hidraw devices give us full reports or not, which I haven't investigated yet. I'm perfectly fine with leaving the test unfixed for now, and have the spurious failures, it just looked bad for my additional tests ;) -- Rémi Bernon <rbernon(a)codeweavers.com>
On 6/3/21 10:50 AM, Rémi Bernon wrote:
On 6/3/21 5:44 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 10:42 AM, Rémi Bernon wrote:
On 6/3/21 5:40 PM, Zebediah Figura (she/her) wrote:
On 6/3/21 7:55 AM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> ---
v2: * Fix spurious failures first.
* Don't try NULL preparsed data, it crashes old windows versions.
* Squash new HidP_Get*Caps tests together, add a few more with specific caps lookup.
dlls/ntoskrnl.exe/tests/driver_hid.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/dlls/ntoskrnl.exe/tests/driver_hid.c b/dlls/ntoskrnl.exe/tests/driver_hid.c index 44ec1b09526..79b3e708752 100644 --- a/dlls/ntoskrnl.exe/tests/driver_hid.c +++ b/dlls/ntoskrnl.exe/tests/driver_hid.c @@ -103,6 +103,7 @@ static const unsigned char report_descriptor[] = static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) { + static int test_failed = 0; IO_STACK_LOCATION *stack = IoGetCurrentIrpStackLocation(irp); const ULONG in_size = stack->Parameters.DeviceIoControl.InputBufferLength; const ULONG out_size = stack->Parameters.DeviceIoControl.OutputBufferLength; @@ -176,7 +177,7 @@ static NTSTATUS WINAPI driver_internal_ioctl(DEVICE_OBJECT *device, IRP *irp) case IOCTL_HID_READ_REPORT: ok(!in_size, "got input size %u\n", in_size); - todo_wine ok(out_size == 2, "got output size %u\n", out_size); + if (test_failed++ < 10) todo_wine ok(out_size == 2, "got output size %u\n", out_size); ret = STATUS_NOT_IMPLEMENTED; break;
Does this test fail on Windows? Can you provide a link to the test results?
It's not failing on windows, but the todo_wine are spamming the test results, making the test be reported as failing because too much data is printed.
Shame on me for introducing that test and never fixing it, but maybe we could just fix it the right way instead?
Yes but it's a little bit more complicated than it seems, and I'm a bit afraid of the possible unforeseen consequences.
There's apparently several ways to request reports (in polled and non-polled modes for instance). and I couldn't test the IOCTL_HID_GET_INPUT_REPORT way yet for instance, I don't know if it passes a packet with the full report length, including 0 report id or if it uses a different length depending on when report ids are used or not.
Also in winebus.sys, both ways are handled by deliver_last_report, which expects a full report.
Then there's the question whether reading hidraw devices give us full reports or not, which I haven't investigated yet.
I'm perfectly fine with leaving the test unfixed for now, and have the spurious failures, it just looked bad for my additional tests ;)
Okay, I guess I'm fine with it too, I might recommend really only incrementing test_failure on failure, though, and maybe there's no reason to set the limit at 10 instead of 1.
participants (3)
-
Marvin -
Rémi Bernon -
Zebediah Figura (she/her)