On Wed Sep 10 18:11:22 2025 +0000, Elizabeth Figura wrote:
7/10:
- ok(io.Status == params->iosb_status, "got %#lx\n", io.Status); - ok(io.Information == 3, "got size %Iu\n", io.Information); + todo_wine_if (NT_ERROR(params->iosb_status) && params->pending) + ok(io.Status == params->iosb_status || broken(NT_ERROR(params->iosb_status) && params->pending), "got %#lx\n", io.Status); + todo_wine_if (NT_ERROR(params->iosb_status) && params->pending) + ok(io.Information == 3 || broken(NT_ERROR(params->iosb_status) && params->pending), "got size %Iu\n", io.Information);We should also be testing the actual values in those broken() blocks. Also, can we please mark which versions have which results in a comment?
ret = NtDeviceIoControlFile(file, event, NULL, (void *)456, &io, ioctl, params, sizeof(*params), buffer, sizeof(buffer)); ok(ret == params->ret_status - || broken(NT_WARNING(params->ret_status) && ret == STATUS_PENDING), /* win10 */ + || (NT_WARNING(params->ret_status) && ret == STATUS_PENDING), "got %#x\n", ret); if (!params->pending && NT_ERROR(params->iosb_status)) {Similarly, no objection to removing broken(), but can we please keep the comment?
- ok(io.Status == 0xdeadf00d, "got iosb status %#lx\n", io.Status); - ok(io.Information == 0xdeadf00d, "got information %#Ix\n", io.Information); + todo_wine ok(io.Status == 0xc00000a3 || broken(io.Status == 0xdeadf00d), /* older win10 */ + "got iosb status %#lx\n", io.Status); + todo_wine ok(io.Information == 0 || broken(io.Information == 0xdeadf00d), /* older win10 */ + "got information %#Ix\n", io.Information);c00000a3 is STATUS_DEVICE_NOT_READY. 8/10:
+ case IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE_INIT: + KeInitializeDeviceQueue(&device_queue); + ok(IsListEmpty(&device_queue.DeviceListHead), "expected empty queue list\n"); + status = STATUS_SUCCESS; + break;Or just initialize it once in DriverEntry(), and then you don't need a dedicated ioctl.
+#define IOCTL_WINETEST_TEST_CANCEL_UNCANCELLABLE CTL_CODE(FILE_DEVICE_UNKNOWN, 0x806, METHOD_BUFFERED, FILE_ANY_ACCESS) +#define IOCTL_WINETEST_TEST_CANCEL_COMPLETE CTL_CODE(FILE_DEVICE_UNKNOWN, 0x807, METHOD_BUFFERED, FILE_ANY_ACCESS)I'd be inclined to just make these IOCTL_WINETEST_QUEUE_ASYNC and IOCTL_WINETEST_COMPLETE_ASYNC, since they don't necessarily have anything to do with cancellation, and not upend the rest of the definitions...
+ /* put the IRP into the queue */ + ret = KeInsertDeviceQueue(&device_queue, &irp->Tail.Overlay.DeviceQueueEntry); + /* or store it elsewhere if it's the one we're supposing to be + * working on "currently" */ + if (!ret) + current_irp = irp;KDEVICE_QUEUE is an odd API and I feel like it honestly makes this test more confusing. I'd just have a simple "queued_async_irps[2]" array and stash them there. Incidentally, because there is some loose threading involved I would be inclined to validate everything we can from the kernel side. Make sure there really is an IRP queued before we complete it, etc. I know I usually say "there's no use avoiding a crash; a failure is a failure", but bluescreens are a different story, because they make iterating a pain.
+ returned = FALSE; + ctx.file = file; + ctx.ex = FALSE; + thread = CreateThread(NULL, 0, test_cancel_complete_thread, &ctx, 0, NULL); + res = CancelIo(file); + ok(res, "CancelIo failed: %lu\n", GetLastError()); + returned = TRUE; + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread);The way I tend to prefer writing this kind of test is to do the blocking action from the thread, check (from the main thread) that the thread doesn't complete immediately (e.g. WaitForSingleObject(200)), and then perform the action from the main thread that should complete it and check that it completes within another reasonable amount of time. I think it's a little easier to read. If you feel this is fine though I won't object.
Alright, I think I fixed up all of the above. I have some comments though...
I don't have particularly good "versions" to put in the `broken` test comments in patch 7/10. Basically, current Windows 10 AND 11's behavior is different from older Windows 10 / 11. Also, for Windows 10 at least, we're talking about the same 22H2 (or 19045) version.
There is a "revision" value accessible through the registry which can help a bit (see e.g. https://learn.microsoft.com/en-us/windows/release-health/release-information, that's the value after the dot under the Build column) Notice also that those revision numbers are specific for each different OS version and can, and will, clash among different versions. I have a small patch to print that value from winetest, since it seems to be generally useful information to have.
That doesn't help a ton here though. I know that the newest Win10 revision on the testbot and test.winehq.org (19045.2311) has the "old" ntoskrnl tests behavior, while my own 19045.6216 has the "new" one. I could make the comments a bit more thorough but I'm not sure that's much of an improvement.