[PATCH v2 1/4] hid/tests: Call CancelIo before closing device.
This was causing some leak to be reported on wineserver shutdown when no data can be read from a device. Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/tests/device.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c index bb819cc83f2..19f450045f1 100644 --- a/dlls/hid/tests/device.c +++ b/dlls/hid/tests/device.c @@ -316,6 +316,7 @@ static void test_read_device(void) CloseHandle(overlapped.hEvent); rc = HidD_FreePreparsedData(ppd); ok(rc, "Failed to free preparsed data(0x%x)\n", GetLastError()); + CancelIo(device); CloseHandle(device); HeapFree(GetProcessHeap(), 0, data); HeapFree(GetProcessHeap(), 0, report); -- 2.25.0
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/hid.spec | 2 +- dlls/hid/hidp.c | 10 ++++++++++ include/ddk/hidpi.h | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/dlls/hid/hid.spec b/dlls/hid/hid.spec index b50e04d197f..b8e29fef16a 100644 --- a/dlls/hid/hid.spec +++ b/dlls/hid/hid.spec @@ -22,7 +22,7 @@ @ stdcall HidP_GetCaps(ptr ptr) @ stdcall HidP_GetData(long ptr ptr ptr ptr long) @ stub HidP_GetExtendedAttributes -@ stub HidP_GetLinkCollectionNodes +@ stdcall HidP_GetLinkCollectionNodes(ptr ptr ptr) @ stdcall HidP_GetScaledUsageValue(long long long long ptr ptr ptr long) @ stdcall HidP_GetSpecificButtonCaps(long long long long ptr ptr ptr) @ stdcall HidP_GetSpecificValueCaps(long long long long ptr ptr ptr) diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c index 9d631912c85..e0513af7fba 100644 --- a/dlls/hid/hidp.c +++ b/dlls/hid/hidp.c @@ -927,3 +927,13 @@ NTSTATUS WINAPI HidP_GetData(HIDP_REPORT_TYPE ReportType, HIDP_DATA *DataList, U return rc; } + +NTSTATUS WINAPI HidP_GetLinkCollectionNodes(HIDP_LINK_COLLECTION_NODE *LinkCollectionNode, + ULONG *LinkCollectionNodeLength, PHIDP_PREPARSED_DATA PreparsedData) +{ + FIXME("stub (%p, %p, %p)\n", LinkCollectionNode, LinkCollectionNodeLength, PreparsedData); + + *LinkCollectionNodeLength = 0; + + return STATUS_NOT_IMPLEMENTED; +} diff --git a/include/ddk/hidpi.h b/include/ddk/hidpi.h index 042c84dd1c5..51d61ea8541 100644 --- a/include/ddk/hidpi.h +++ b/include/ddk/hidpi.h @@ -175,6 +175,19 @@ typedef struct _HIDP_DATA { } DUMMYUNIONNAME; } HIDP_DATA, *PHIDP_DATA; +typedef struct _HIDP_LINK_COLLECTION_NODE { + USAGE LinkUsage; + USAGE LinkUsagePage; + USHORT Parent; + USHORT NumberOfChildren; + USHORT NextSibling; + USHORT FirstChild; + ULONG CollectionType : 8; + ULONG IsAlias : 1; + ULONG Reserved : 23; + PVOID UserContext; +} HIDP_LINK_COLLECTION_NODE, *PHIDP_LINK_COLLECTION_NODE; + typedef BOOLEAN (NTAPI *PHIDP_INSERT_SCANCODES) (VOID *Context, CHAR *NewScanCodes, ULONG Length); @@ -196,6 +209,7 @@ NTSTATUS WINAPI HidP_GetSpecificValueCaps(HIDP_REPORT_TYPE ReportType, USAGE Usa NTSTATUS WINAPI HidP_GetUsagesEx(HIDP_REPORT_TYPE ReportType, USHORT LinkCollection, USAGE_AND_PAGE *ButtonList, ULONG *UsageLength, PHIDP_PREPARSED_DATA PreparsedData, CHAR *Report, ULONG ReportLength); ULONG WINAPI HidP_MaxDataListLength(HIDP_REPORT_TYPE ReportType, PHIDP_PREPARSED_DATA PreparsedData); NTSTATUS WINAPI HidP_GetData(HIDP_REPORT_TYPE ReportType, HIDP_DATA *DataList, ULONG *DataLength, PHIDP_PREPARSED_DATA PreparsedData, CHAR *Report, ULONG ReportLength); +NTSTATUS WINAPI HidP_GetLinkCollectionNodes(HIDP_LINK_COLLECTION_NODE *LinkCollectionNode, ULONG *LinkCollectionNodeLength, PHIDP_PREPARSED_DATA PreparsedData); #ifndef FACILITY_HID_ERROR_CODE #define FACILITY_HID_ERROR_CODE 0x11 -- 2.25.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=64932 Your paranoid android. === debian10 (32 bit report) === === debian10 (32 bit Chinese:China report) === === debian10 (32 bit WoW report) === === debian10 (64 bit WoW report) ===
Signed-off-by: Aric Stewart <aric(a)codeweavers.com> On 2/11/20 12:13 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/hid.spec | 2 +- dlls/hid/hidp.c | 10 ++++++++++ include/ddk/hidpi.h | 14 ++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-)
diff --git a/dlls/hid/hid.spec b/dlls/hid/hid.spec index b50e04d197f..b8e29fef16a 100644 --- a/dlls/hid/hid.spec +++ b/dlls/hid/hid.spec @@ -22,7 +22,7 @@ @ stdcall HidP_GetCaps(ptr ptr) @ stdcall HidP_GetData(long ptr ptr ptr ptr long) @ stub HidP_GetExtendedAttributes -@ stub HidP_GetLinkCollectionNodes +@ stdcall HidP_GetLinkCollectionNodes(ptr ptr ptr) @ stdcall HidP_GetScaledUsageValue(long long long long ptr ptr ptr long) @ stdcall HidP_GetSpecificButtonCaps(long long long long ptr ptr ptr) @ stdcall HidP_GetSpecificValueCaps(long long long long ptr ptr ptr) diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c index 9d631912c85..e0513af7fba 100644 --- a/dlls/hid/hidp.c +++ b/dlls/hid/hidp.c @@ -927,3 +927,13 @@ NTSTATUS WINAPI HidP_GetData(HIDP_REPORT_TYPE ReportType, HIDP_DATA *DataList, U
return rc; } + +NTSTATUS WINAPI HidP_GetLinkCollectionNodes(HIDP_LINK_COLLECTION_NODE *LinkCollectionNode, + ULONG *LinkCollectionNodeLength, PHIDP_PREPARSED_DATA PreparsedData) +{ + FIXME("stub (%p, %p, %p)\n", LinkCollectionNode, LinkCollectionNodeLength, PreparsedData); + + *LinkCollectionNodeLength = 0; + + return STATUS_NOT_IMPLEMENTED; +} diff --git a/include/ddk/hidpi.h b/include/ddk/hidpi.h index 042c84dd1c5..51d61ea8541 100644 --- a/include/ddk/hidpi.h +++ b/include/ddk/hidpi.h @@ -175,6 +175,19 @@ typedef struct _HIDP_DATA { } DUMMYUNIONNAME; } HIDP_DATA, *PHIDP_DATA;
+typedef struct _HIDP_LINK_COLLECTION_NODE { + USAGE LinkUsage; + USAGE LinkUsagePage; + USHORT Parent; + USHORT NumberOfChildren; + USHORT NextSibling; + USHORT FirstChild; + ULONG CollectionType : 8; + ULONG IsAlias : 1; + ULONG Reserved : 23; + PVOID UserContext; +} HIDP_LINK_COLLECTION_NODE, *PHIDP_LINK_COLLECTION_NODE; + typedef BOOLEAN (NTAPI *PHIDP_INSERT_SCANCODES) (VOID *Context, CHAR *NewScanCodes, ULONG Length);
@@ -196,6 +209,7 @@ NTSTATUS WINAPI HidP_GetSpecificValueCaps(HIDP_REPORT_TYPE ReportType, USAGE Usa NTSTATUS WINAPI HidP_GetUsagesEx(HIDP_REPORT_TYPE ReportType, USHORT LinkCollection, USAGE_AND_PAGE *ButtonList, ULONG *UsageLength, PHIDP_PREPARSED_DATA PreparsedData, CHAR *Report, ULONG ReportLength); ULONG WINAPI HidP_MaxDataListLength(HIDP_REPORT_TYPE ReportType, PHIDP_PREPARSED_DATA PreparsedData); NTSTATUS WINAPI HidP_GetData(HIDP_REPORT_TYPE ReportType, HIDP_DATA *DataList, ULONG *DataLength, PHIDP_PREPARSED_DATA PreparsedData, CHAR *Report, ULONG ReportLength); +NTSTATUS WINAPI HidP_GetLinkCollectionNodes(HIDP_LINK_COLLECTION_NODE *LinkCollectionNode, ULONG *LinkCollectionNodeLength, PHIDP_PREPARSED_DATA PreparsedData);
#ifndef FACILITY_HID_ERROR_CODE #define FACILITY_HID_ERROR_CODE 0x11
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/tests/device.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c index 19f450045f1..b3e959d2594 100644 --- a/dlls/hid/tests/device.c +++ b/dlls/hid/tests/device.c @@ -35,9 +35,12 @@ static void test_device_info(HANDLE device) PHIDP_PREPARSED_DATA ppd; HIDP_CAPS Caps; HIDD_ATTRIBUTES attributes; + HIDP_LINK_COLLECTION_NODE nodes[16]; + ULONG nodes_count; NTSTATUS status; BOOL rc; WCHAR device_name[128]; + int i; rc = HidD_GetPreparsedData(device, &ppd); ok(rc, "Failed to get preparsed data(0x%x)\n", GetLastError()); @@ -46,6 +49,40 @@ static void test_device_info(HANDLE device) rc = HidD_GetProductString(device, device_name, sizeof(device_name)); ok(rc, "Failed to get product string(0x%x)\n", GetLastError()); trace("Found device %s (%02x, %02x)\n", wine_dbgstr_w(device_name), Caps.UsagePage, Caps.Usage); + + trace("LinkCollectionNodes: (%d)\n", Caps.NumberLinkCollectionNodes); + todo_wine + ok(Caps.NumberLinkCollectionNodes > 0, "Expected at least one link collection\n"); + + nodes_count = 0; + status = HidP_GetLinkCollectionNodes(nodes, &nodes_count, ppd); + todo_wine + ok(status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetLinkCollectionNodes succeeded:%x\n", status); + + nodes_count = ARRAY_SIZE(nodes); + status = HidP_GetLinkCollectionNodes(nodes, &nodes_count, ppd); + todo_wine + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetLinkCollectionNodes failed:%x\n", status); + + for (i = 0; i < nodes_count; ++i) + { + trace(" [%d] LinkUsage: %x LinkUsagePage: %x Parent: %x " + "NumberOfChildren: %x NextSibling: %x FirstChild: %x " + "CollectionType: %x IsAlias: %x UserContext: %p\n", + i, nodes[i].LinkUsage, nodes[i].LinkUsagePage, nodes[i].Parent, + nodes[i].NumberOfChildren, nodes[i].NextSibling, nodes[i].FirstChild, + nodes[i].CollectionType, nodes[i].IsAlias, nodes[i].UserContext); + } + + todo_wine + ok(nodes_count > 0, "Unexpected number of link collection nodes:%u.\n", nodes_count); + todo_wine + ok(nodes[0].LinkUsagePage == Caps.UsagePage, "Unexpected top collection usage page:%x\n", nodes[0].LinkUsagePage); + todo_wine + ok(nodes[0].LinkUsage == Caps.Usage, "Unexpected top collection usage:%x\n", nodes[0].LinkUsage); + todo_wine + ok(nodes[0].CollectionType == 1, "Unexpected top collection type:%x\n", nodes[0].CollectionType); + rc = HidD_FreePreparsedData(ppd); ok(rc, "Failed to free preparsed data(0x%x)\n", GetLastError()); rc = HidD_GetAttributes(device, &attributes); -- 2.25.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=64933 Your paranoid android. === wxppro (32 bit report) === === w2008s64 (32 bit report) === === w8 (32 bit report) === === w8adm (32 bit report) === === w864 (32 bit report) === Report validation errors: hid:device has no test summary line (early exit of the main process?) === w1064v1507 (32 bit report) === === w1064v1809 (32 bit report) === === w1064v1809_2scr (32 bit report) === === w1064v1809_ar (32 bit report) === === w1064v1809_he (32 bit report) === Report validation errors: hid:device has no test summary line (early exit of the main process?) === w1064v1809_ja (32 bit report) === Report validation errors: hid:device has no test summary line (early exit of the main process?) === w1064v1809_zh_CN (32 bit report) === === w2008s64 (64 bit report) === === w864 (64 bit report) === === w1064v1507 (64 bit report) === === w1064v1809 (64 bit report) === === debian10 (32 bit report) === === debian10 (32 bit French report) === === debian10 (32 bit Japanese:Japan report) === === debian10 (32 bit Chinese:China report) === === debian10 (32 bit WoW report) === === debian10 (64 bit WoW report) ===
Signed-off-by: Aric Stewart <aric(a)codeweavers.com> On 2/11/20 12:13 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/tests/device.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c index 19f450045f1..b3e959d2594 100644 --- a/dlls/hid/tests/device.c +++ b/dlls/hid/tests/device.c @@ -35,9 +35,12 @@ static void test_device_info(HANDLE device) PHIDP_PREPARSED_DATA ppd; HIDP_CAPS Caps; HIDD_ATTRIBUTES attributes; + HIDP_LINK_COLLECTION_NODE nodes[16]; + ULONG nodes_count; NTSTATUS status; BOOL rc; WCHAR device_name[128]; + int i;
rc = HidD_GetPreparsedData(device, &ppd); ok(rc, "Failed to get preparsed data(0x%x)\n", GetLastError()); @@ -46,6 +49,40 @@ static void test_device_info(HANDLE device) rc = HidD_GetProductString(device, device_name, sizeof(device_name)); ok(rc, "Failed to get product string(0x%x)\n", GetLastError()); trace("Found device %s (%02x, %02x)\n", wine_dbgstr_w(device_name), Caps.UsagePage, Caps.Usage); + + trace("LinkCollectionNodes: (%d)\n", Caps.NumberLinkCollectionNodes); + todo_wine + ok(Caps.NumberLinkCollectionNodes > 0, "Expected at least one link collection\n"); + + nodes_count = 0; + status = HidP_GetLinkCollectionNodes(nodes, &nodes_count, ppd); + todo_wine + ok(status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetLinkCollectionNodes succeeded:%x\n", status); + + nodes_count = ARRAY_SIZE(nodes); + status = HidP_GetLinkCollectionNodes(nodes, &nodes_count, ppd); + todo_wine + ok(status == HIDP_STATUS_SUCCESS, "HidP_GetLinkCollectionNodes failed:%x\n", status); + + for (i = 0; i < nodes_count; ++i) + { + trace(" [%d] LinkUsage: %x LinkUsagePage: %x Parent: %x " + "NumberOfChildren: %x NextSibling: %x FirstChild: %x " + "CollectionType: %x IsAlias: %x UserContext: %p\n", + i, nodes[i].LinkUsage, nodes[i].LinkUsagePage, nodes[i].Parent, + nodes[i].NumberOfChildren, nodes[i].NextSibling, nodes[i].FirstChild, + nodes[i].CollectionType, nodes[i].IsAlias, nodes[i].UserContext); + } + + todo_wine + ok(nodes_count > 0, "Unexpected number of link collection nodes:%u.\n", nodes_count); + todo_wine + ok(nodes[0].LinkUsagePage == Caps.UsagePage, "Unexpected top collection usage page:%x\n", nodes[0].LinkUsagePage); + todo_wine + ok(nodes[0].LinkUsage == Caps.Usage, "Unexpected top collection usage:%x\n", nodes[0].LinkUsage); + todo_wine + ok(nodes[0].CollectionType == 1, "Unexpected top collection type:%x\n", nodes[0].CollectionType); + rc = HidD_FreePreparsedData(ppd); ok(rc, "Failed to free preparsed data(0x%x)\n", GetLastError()); rc = HidD_GetAttributes(device, &attributes);
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/hidp.c | 24 ++++++++++++++++++++--- dlls/hid/tests/device.c | 7 ------- dlls/hidclass.sys/descriptor.c | 36 +++++++++++++++++++++++++++++----- include/wine/hid.h | 13 ++++++++++++ 4 files changed, 65 insertions(+), 15 deletions(-) diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c index e0513af7fba..817e021fe4e 100644 --- a/dlls/hid/hidp.c +++ b/dlls/hid/hidp.c @@ -931,9 +931,27 @@ NTSTATUS WINAPI HidP_GetData(HIDP_REPORT_TYPE ReportType, HIDP_DATA *DataList, U NTSTATUS WINAPI HidP_GetLinkCollectionNodes(HIDP_LINK_COLLECTION_NODE *LinkCollectionNode, ULONG *LinkCollectionNodeLength, PHIDP_PREPARSED_DATA PreparsedData) { - FIXME("stub (%p, %p, %p)\n", LinkCollectionNode, LinkCollectionNodeLength, PreparsedData); + WINE_HIDP_PREPARSED_DATA *data = (WINE_HIDP_PREPARSED_DATA*)PreparsedData; + WINE_HID_LINK_COLLECTION_NODE *nodes = HID_NODES(data); + ULONG i; - *LinkCollectionNodeLength = 0; + TRACE("(%p, %p, %p)\n", LinkCollectionNode, LinkCollectionNodeLength, PreparsedData); - return STATUS_NOT_IMPLEMENTED; + if (*LinkCollectionNodeLength < data->caps.NumberLinkCollectionNodes) + return HIDP_STATUS_BUFFER_TOO_SMALL; + + for (i = 0; i < data->caps.NumberLinkCollectionNodes; ++i) + { + LinkCollectionNode[i].LinkUsage = nodes[i].LinkUsage; + LinkCollectionNode[i].LinkUsagePage = nodes[i].LinkUsagePage; + LinkCollectionNode[i].Parent = nodes[i].Parent; + LinkCollectionNode[i].NumberOfChildren = nodes[i].NumberOfChildren; + LinkCollectionNode[i].NextSibling = nodes[i].NextSibling; + LinkCollectionNode[i].FirstChild = nodes[i].FirstChild; + LinkCollectionNode[i].CollectionType = nodes[i].CollectionType; + LinkCollectionNode[i].IsAlias = nodes[i].IsAlias; + } + *LinkCollectionNodeLength = data->caps.NumberLinkCollectionNodes; + + return HIDP_STATUS_SUCCESS; } diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c index b3e959d2594..24c307704fe 100644 --- a/dlls/hid/tests/device.c +++ b/dlls/hid/tests/device.c @@ -51,17 +51,14 @@ static void test_device_info(HANDLE device) trace("Found device %s (%02x, %02x)\n", wine_dbgstr_w(device_name), Caps.UsagePage, Caps.Usage); trace("LinkCollectionNodes: (%d)\n", Caps.NumberLinkCollectionNodes); - todo_wine ok(Caps.NumberLinkCollectionNodes > 0, "Expected at least one link collection\n"); nodes_count = 0; status = HidP_GetLinkCollectionNodes(nodes, &nodes_count, ppd); - todo_wine ok(status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetLinkCollectionNodes succeeded:%x\n", status); nodes_count = ARRAY_SIZE(nodes); status = HidP_GetLinkCollectionNodes(nodes, &nodes_count, ppd); - todo_wine ok(status == HIDP_STATUS_SUCCESS, "HidP_GetLinkCollectionNodes failed:%x\n", status); for (i = 0; i < nodes_count; ++i) @@ -74,13 +71,9 @@ static void test_device_info(HANDLE device) nodes[i].CollectionType, nodes[i].IsAlias, nodes[i].UserContext); } - todo_wine ok(nodes_count > 0, "Unexpected number of link collection nodes:%u.\n", nodes_count); - todo_wine ok(nodes[0].LinkUsagePage == Caps.UsagePage, "Unexpected top collection usage page:%x\n", nodes[0].LinkUsagePage); - todo_wine ok(nodes[0].LinkUsage == Caps.Usage, "Unexpected top collection usage:%x\n", nodes[0].LinkUsage); - todo_wine ok(nodes[0].CollectionType == 1, "Unexpected top collection type:%x\n", nodes[0].CollectionType); rc = HidD_FreePreparsedData(ppd); diff --git a/dlls/hidclass.sys/descriptor.c b/dlls/hidclass.sys/descriptor.c index af1679d23b0..876369ad701 100644 --- a/dlls/hidclass.sys/descriptor.c +++ b/dlls/hidclass.sys/descriptor.c @@ -835,12 +835,14 @@ static void create_preparse_ctx(const struct collection *base, struct preparse_c create_preparse_ctx(c, ctx); } -static void preparse_collection(const struct collection *base, +static void preparse_collection(const struct collection *root, const struct collection *base, WINE_HIDP_PREPARSED_DATA *data, struct preparse_ctx *ctx) { WINE_HID_ELEMENT *elem = HID_ELEMS(data); + WINE_HID_LINK_COLLECTION_NODE *nodes = HID_NODES(data); struct feature *f; struct collection *c; + struct list *entry; LIST_FOR_EACH_ENTRY(f, &base->features, struct feature, entry) { @@ -885,11 +887,29 @@ static void preparse_collection(const struct collection *base, } } + if (root != base) + { + nodes[base->index].LinkUsagePage = base->caps.UsagePage; + nodes[base->index].LinkUsage = base->caps.u.NotRange.Usage; + nodes[base->index].Parent = base->parent == root ? 0 : base->parent->index; + nodes[base->index].CollectionType = base->type; + nodes[base->index].IsAlias = 0; + + if ((entry = list_head(&base->collections))) + nodes[base->index].FirstChild = LIST_ENTRY(entry, struct collection, entry)->index; + } + LIST_FOR_EACH_ENTRY(c, &base->collections, struct collection, entry) - preparse_collection(c, data, ctx); + { + preparse_collection(root, c, data, ctx); + + if ((entry = list_next(&base->collections, &c->entry))) + nodes[c->index].NextSibling = LIST_ENTRY(entry, struct collection, entry)->index; + if (root != base) nodes[base->index].NumberOfChildren++; + } } -static WINE_HIDP_PREPARSED_DATA* build_PreparseData(struct collection *base_collection) +static WINE_HIDP_PREPARSED_DATA* build_PreparseData(struct collection *base_collection, unsigned int node_count) { WINE_HIDP_PREPARSED_DATA *data; unsigned int report_count; @@ -897,6 +917,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData(struct collection *base_coll struct preparse_ctx ctx; unsigned int element_off; + unsigned int nodes_offset; memset(&ctx, 0, sizeof(ctx)); create_preparse_ctx(base_collection, &ctx); @@ -906,14 +927,19 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData(struct collection *base_coll element_off = FIELD_OFFSET(WINE_HIDP_PREPARSED_DATA, reports[report_count]); size = element_off + (ctx.elem_count * sizeof(WINE_HID_ELEMENT)); + nodes_offset = size; + size += node_count * sizeof(WINE_HID_LINK_COLLECTION_NODE); + data = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size); data->magic = HID_MAGIC; data->dwSize = size; data->caps.Usage = base_collection->caps.u.NotRange.Usage; data->caps.UsagePage = base_collection->caps.UsagePage; + data->caps.NumberLinkCollectionNodes = node_count; data->elementOffset = element_off; + data->nodesOffset = nodes_offset; - preparse_collection(base_collection, data, &ctx); + preparse_collection(base_collection, base_collection, data, &ctx); return data; } @@ -981,7 +1007,7 @@ WINE_HIDP_PREPARSED_DATA* ParseDescriptor(BYTE *descriptor, unsigned int length) } } - data = build_PreparseData(base); + data = build_PreparseData(base, cidx); debug_print_preparsed(data); free_collection(base); diff --git a/include/wine/hid.h b/include/wine/hid.h index 319105d286e..857dcae18fe 100644 --- a/include/wine/hid.h +++ b/include/wine/hid.h @@ -48,6 +48,17 @@ typedef struct __WINE_HID_REPORT DWORD elementIdx; } WINE_HID_REPORT; +typedef struct __WINE_HID_LINK_COLLECTION_NODE { + USAGE LinkUsage; + USAGE LinkUsagePage; + USHORT Parent; + USHORT NumberOfChildren; + USHORT NextSibling; + USHORT FirstChild; + BYTE CollectionType; + BYTE IsAlias; +} WINE_HID_LINK_COLLECTION_NODE; + typedef struct __WINE_HIDP_PREPARSED_DATA { DWORD magic; @@ -55,6 +66,7 @@ typedef struct __WINE_HIDP_PREPARSED_DATA HIDP_CAPS caps; DWORD elementOffset; + DWORD nodesOffset; DWORD reportCount[3]; BYTE reportIdx[3][256]; @@ -65,5 +77,6 @@ typedef struct __WINE_HIDP_PREPARSED_DATA #define HID_OUTPUT_REPORTS(d) ((d)->reports + (d)->reportCount[0]) #define HID_FEATURE_REPORTS(d) ((d)->reports + (d)->reportCount[0] + (d)->reportCount[1]) #define HID_ELEMS(d) ((WINE_HID_ELEMENT*)((BYTE*)(d) + (d)->elementOffset)) +#define HID_NODES(d) ((WINE_HID_LINK_COLLECTION_NODE*)((BYTE*)(d) + (d)->nodesOffset)) #endif /* __WINE_PARSE_H */ -- 2.25.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=64934 Your paranoid android. === wxppro (32 bit report) === === w2008s64 (32 bit report) === === w8 (32 bit report) === === w8adm (32 bit report) === === w864 (32 bit report) === Report validation errors: hid:device has no test summary line (early exit of the main process?) === w1064v1507 (32 bit report) === === w1064v1809 (32 bit report) === === w1064v1809_2scr (32 bit report) === === w1064v1809_ar (32 bit report) === === w1064v1809_he (32 bit report) === Report validation errors: hid:device has no test summary line (early exit of the main process?) === w1064v1809_ja (32 bit report) === Report validation errors: hid:device has no test summary line (early exit of the main process?) === w1064v1809_zh_CN (32 bit report) === === w2008s64 (64 bit report) === === w864 (64 bit report) === === w1064v1507 (64 bit report) === === w1064v1809 (64 bit report) === === debian10 (32 bit report) === === debian10 (32 bit French report) === === debian10 (32 bit Japanese:Japan report) === === debian10 (32 bit Chinese:China report) === === debian10 (32 bit WoW report) === === debian10 (64 bit WoW report) ===
Signed-off-by: Aric Stewart <aric(a)codeweavers.com> On 2/11/20 12:13 PM, Rémi Bernon wrote:
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/hidp.c | 24 ++++++++++++++++++++--- dlls/hid/tests/device.c | 7 ------- dlls/hidclass.sys/descriptor.c | 36 +++++++++++++++++++++++++++++----- include/wine/hid.h | 13 ++++++++++++ 4 files changed, 65 insertions(+), 15 deletions(-)
diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c index e0513af7fba..817e021fe4e 100644 --- a/dlls/hid/hidp.c +++ b/dlls/hid/hidp.c @@ -931,9 +931,27 @@ NTSTATUS WINAPI HidP_GetData(HIDP_REPORT_TYPE ReportType, HIDP_DATA *DataList, U NTSTATUS WINAPI HidP_GetLinkCollectionNodes(HIDP_LINK_COLLECTION_NODE *LinkCollectionNode, ULONG *LinkCollectionNodeLength, PHIDP_PREPARSED_DATA PreparsedData) { - FIXME("stub (%p, %p, %p)\n", LinkCollectionNode, LinkCollectionNodeLength, PreparsedData); + WINE_HIDP_PREPARSED_DATA *data = (WINE_HIDP_PREPARSED_DATA*)PreparsedData; + WINE_HID_LINK_COLLECTION_NODE *nodes = HID_NODES(data); + ULONG i;
- *LinkCollectionNodeLength = 0; + TRACE("(%p, %p, %p)\n", LinkCollectionNode, LinkCollectionNodeLength, PreparsedData);
- return STATUS_NOT_IMPLEMENTED; + if (*LinkCollectionNodeLength < data->caps.NumberLinkCollectionNodes) + return HIDP_STATUS_BUFFER_TOO_SMALL; + + for (i = 0; i < data->caps.NumberLinkCollectionNodes; ++i) + { + LinkCollectionNode[i].LinkUsage = nodes[i].LinkUsage; + LinkCollectionNode[i].LinkUsagePage = nodes[i].LinkUsagePage; + LinkCollectionNode[i].Parent = nodes[i].Parent; + LinkCollectionNode[i].NumberOfChildren = nodes[i].NumberOfChildren; + LinkCollectionNode[i].NextSibling = nodes[i].NextSibling; + LinkCollectionNode[i].FirstChild = nodes[i].FirstChild; + LinkCollectionNode[i].CollectionType = nodes[i].CollectionType; + LinkCollectionNode[i].IsAlias = nodes[i].IsAlias; + } + *LinkCollectionNodeLength = data->caps.NumberLinkCollectionNodes; + + return HIDP_STATUS_SUCCESS; } diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c index b3e959d2594..24c307704fe 100644 --- a/dlls/hid/tests/device.c +++ b/dlls/hid/tests/device.c @@ -51,17 +51,14 @@ static void test_device_info(HANDLE device) trace("Found device %s (%02x, %02x)\n", wine_dbgstr_w(device_name), Caps.UsagePage, Caps.Usage);
trace("LinkCollectionNodes: (%d)\n", Caps.NumberLinkCollectionNodes); - todo_wine ok(Caps.NumberLinkCollectionNodes > 0, "Expected at least one link collection\n");
nodes_count = 0; status = HidP_GetLinkCollectionNodes(nodes, &nodes_count, ppd); - todo_wine ok(status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetLinkCollectionNodes succeeded:%x\n", status);
nodes_count = ARRAY_SIZE(nodes); status = HidP_GetLinkCollectionNodes(nodes, &nodes_count, ppd); - todo_wine ok(status == HIDP_STATUS_SUCCESS, "HidP_GetLinkCollectionNodes failed:%x\n", status);
for (i = 0; i < nodes_count; ++i) @@ -74,13 +71,9 @@ static void test_device_info(HANDLE device) nodes[i].CollectionType, nodes[i].IsAlias, nodes[i].UserContext); }
- todo_wine ok(nodes_count > 0, "Unexpected number of link collection nodes:%u.\n", nodes_count); - todo_wine ok(nodes[0].LinkUsagePage == Caps.UsagePage, "Unexpected top collection usage page:%x\n", nodes[0].LinkUsagePage); - todo_wine ok(nodes[0].LinkUsage == Caps.Usage, "Unexpected top collection usage:%x\n", nodes[0].LinkUsage); - todo_wine ok(nodes[0].CollectionType == 1, "Unexpected top collection type:%x\n", nodes[0].CollectionType);
rc = HidD_FreePreparsedData(ppd); diff --git a/dlls/hidclass.sys/descriptor.c b/dlls/hidclass.sys/descriptor.c index af1679d23b0..876369ad701 100644 --- a/dlls/hidclass.sys/descriptor.c +++ b/dlls/hidclass.sys/descriptor.c @@ -835,12 +835,14 @@ static void create_preparse_ctx(const struct collection *base, struct preparse_c create_preparse_ctx(c, ctx); }
-static void preparse_collection(const struct collection *base, +static void preparse_collection(const struct collection *root, const struct collection *base, WINE_HIDP_PREPARSED_DATA *data, struct preparse_ctx *ctx) { WINE_HID_ELEMENT *elem = HID_ELEMS(data); + WINE_HID_LINK_COLLECTION_NODE *nodes = HID_NODES(data); struct feature *f; struct collection *c; + struct list *entry;
LIST_FOR_EACH_ENTRY(f, &base->features, struct feature, entry) { @@ -885,11 +887,29 @@ static void preparse_collection(const struct collection *base, } }
+ if (root != base) + { + nodes[base->index].LinkUsagePage = base->caps.UsagePage; + nodes[base->index].LinkUsage = base->caps.u.NotRange.Usage; + nodes[base->index].Parent = base->parent == root ? 0 : base->parent->index; + nodes[base->index].CollectionType = base->type; + nodes[base->index].IsAlias = 0; + + if ((entry = list_head(&base->collections))) + nodes[base->index].FirstChild = LIST_ENTRY(entry, struct collection, entry)->index; + } + LIST_FOR_EACH_ENTRY(c, &base->collections, struct collection, entry) - preparse_collection(c, data, ctx); + { + preparse_collection(root, c, data, ctx); + + if ((entry = list_next(&base->collections, &c->entry))) + nodes[c->index].NextSibling = LIST_ENTRY(entry, struct collection, entry)->index; + if (root != base) nodes[base->index].NumberOfChildren++; + } }
-static WINE_HIDP_PREPARSED_DATA* build_PreparseData(struct collection *base_collection) +static WINE_HIDP_PREPARSED_DATA* build_PreparseData(struct collection *base_collection, unsigned int node_count) { WINE_HIDP_PREPARSED_DATA *data; unsigned int report_count; @@ -897,6 +917,7 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData(struct collection *base_coll
struct preparse_ctx ctx; unsigned int element_off; + unsigned int nodes_offset;
memset(&ctx, 0, sizeof(ctx)); create_preparse_ctx(base_collection, &ctx); @@ -906,14 +927,19 @@ static WINE_HIDP_PREPARSED_DATA* build_PreparseData(struct collection *base_coll element_off = FIELD_OFFSET(WINE_HIDP_PREPARSED_DATA, reports[report_count]); size = element_off + (ctx.elem_count * sizeof(WINE_HID_ELEMENT));
+ nodes_offset = size; + size += node_count * sizeof(WINE_HID_LINK_COLLECTION_NODE); + data = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY, size); data->magic = HID_MAGIC; data->dwSize = size; data->caps.Usage = base_collection->caps.u.NotRange.Usage; data->caps.UsagePage = base_collection->caps.UsagePage; + data->caps.NumberLinkCollectionNodes = node_count; data->elementOffset = element_off; + data->nodesOffset = nodes_offset;
- preparse_collection(base_collection, data, &ctx); + preparse_collection(base_collection, base_collection, data, &ctx); return data; }
@@ -981,7 +1007,7 @@ WINE_HIDP_PREPARSED_DATA* ParseDescriptor(BYTE *descriptor, unsigned int length) } }
- data = build_PreparseData(base); + data = build_PreparseData(base, cidx); debug_print_preparsed(data); free_collection(base);
diff --git a/include/wine/hid.h b/include/wine/hid.h index 319105d286e..857dcae18fe 100644 --- a/include/wine/hid.h +++ b/include/wine/hid.h @@ -48,6 +48,17 @@ typedef struct __WINE_HID_REPORT DWORD elementIdx; } WINE_HID_REPORT;
+typedef struct __WINE_HID_LINK_COLLECTION_NODE { + USAGE LinkUsage; + USAGE LinkUsagePage; + USHORT Parent; + USHORT NumberOfChildren; + USHORT NextSibling; + USHORT FirstChild; + BYTE CollectionType; + BYTE IsAlias; +} WINE_HID_LINK_COLLECTION_NODE; + typedef struct __WINE_HIDP_PREPARSED_DATA { DWORD magic; @@ -55,6 +66,7 @@ typedef struct __WINE_HIDP_PREPARSED_DATA HIDP_CAPS caps;
DWORD elementOffset; + DWORD nodesOffset; DWORD reportCount[3]; BYTE reportIdx[3][256];
@@ -65,5 +77,6 @@ typedef struct __WINE_HIDP_PREPARSED_DATA #define HID_OUTPUT_REPORTS(d) ((d)->reports + (d)->reportCount[0]) #define HID_FEATURE_REPORTS(d) ((d)->reports + (d)->reportCount[0] + (d)->reportCount[1]) #define HID_ELEMS(d) ((WINE_HID_ELEMENT*)((BYTE*)(d) + (d)->elementOffset)) +#define HID_NODES(d) ((WINE_HID_LINK_COLLECTION_NODE*)((BYTE*)(d) + (d)->nodesOffset))
#endif /* __WINE_PARSE_H */
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=64931 Your paranoid android. === wxppro (32 bit report) === === w2008s64 (32 bit report) === === w8 (32 bit report) === === w8adm (32 bit report) === === w864 (32 bit report) === Report validation errors: hid:device has no test summary line (early exit of the main process?) === w1064v1507 (32 bit report) === === w1064v1809 (32 bit report) === === w1064v1809_2scr (32 bit report) === === w1064v1809_ar (32 bit report) === === w1064v1809_he (32 bit report) === Report validation errors: hid:device has no test summary line (early exit of the main process?) === w1064v1809_ja (32 bit report) === Report validation errors: hid:device has no test summary line (early exit of the main process?) === w1064v1809_zh_CN (32 bit report) === === w2008s64 (64 bit report) === === w864 (64 bit report) === === w1064v1507 (64 bit report) === === w1064v1809 (64 bit report) === === debian10 (32 bit report) === === debian10 (32 bit French report) === === debian10 (32 bit Japanese:Japan report) === === debian10 (32 bit Chinese:China report) === === debian10 (32 bit WoW report) === === debian10 (64 bit WoW report) ===
On 2/11/20 12:13 PM, Rémi Bernon wrote:
This was causing some leak to be reported on wineserver shutdown when no data can be read from a device.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/tests/device.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c index bb819cc83f2..19f450045f1 100644 --- a/dlls/hid/tests/device.c +++ b/dlls/hid/tests/device.c @@ -316,6 +316,7 @@ static void test_read_device(void) CloseHandle(overlapped.hEvent); rc = HidD_FreePreparsedData(ppd); ok(rc, "Failed to free preparsed data(0x%x)\n", GetLastError()); + CancelIo(device); CloseHandle(device); HeapFree(GetProcessHeap(), 0, data); HeapFree(GetProcessHeap(), 0, report);
I vaguely recall having seen this leak intermittently as well, and while avoiding the leak in the tests is probably a good idea, I think it's wrong that that leak occurs no matter what "userspace" does. That is, probably some object in wineserver is holding a reference that doesn't release it when it gets destroyed.
Signed-off-by: Aric Stewart <aric(a)codeweavers.com> On 2/11/20 12:13 PM, Rémi Bernon wrote:
This was causing some leak to be reported on wineserver shutdown when no data can be read from a device.
Signed-off-by: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/tests/device.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c index bb819cc83f2..19f450045f1 100644 --- a/dlls/hid/tests/device.c +++ b/dlls/hid/tests/device.c @@ -316,6 +316,7 @@ static void test_read_device(void) CloseHandle(overlapped.hEvent); rc = HidD_FreePreparsedData(ppd); ok(rc, "Failed to free preparsed data(0x%x)\n", GetLastError()); + CancelIo(device); CloseHandle(device); HeapFree(GetProcessHeap(), 0, data); HeapFree(GetProcessHeap(), 0, report);
participants (4)
-
Aric Stewart -
Marvin -
Rémi Bernon -
Zebediah Figura