[PATCH] hid/tests: Add HID device enumeration test
Sebastian Lackner
sebastian at fds-team.de
Mon Oct 17 06:46:44 CDT 2016
On 14.10.2016 11:36, Aric Stewart wrote:
> Signed-off-by: Aric Stewart <aric at codeweavers.com>
> ---
> configure | 1 +
> configure.ac | 1 +
> dlls/hid/tests/Makefile.in | 5 +++
> dlls/hid/tests/device.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 87 insertions(+)
> create mode 100644 dlls/hid/tests/Makefile.in
> create mode 100644 dlls/hid/tests/device.c
>
>
>
> 0001-hid-tests-Add-HID-device-enumeration-test.txt
>
>
> diff --git a/configure b/configure
> index 99793b0..7bb6fef 100755
> --- a/configure
> +++ b/configure
> @@ -17936,6 +17936,7 @@ wine_fn_config_dll gpkcsp enable_gpkcsp
> wine_fn_config_dll hal enable_hal
> wine_fn_config_dll hhctrl.ocx enable_hhctrl_ocx clean,implib htmlhelp
> wine_fn_config_dll hid enable_hid implib
> +wine_fn_config_test dlls/hid/tests hid_test
> wine_fn_config_dll hidclass.sys enable_hidclass_sys implib hidclass
> wine_fn_config_dll hlink enable_hlink clean,implib
> wine_fn_config_test dlls/hlink/tests hlink_test
> diff --git a/configure.ac b/configure.ac
> index ba8fd0f..9cab382 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -3025,6 +3025,7 @@ WINE_CONFIG_DLL(gpkcsp)
> WINE_CONFIG_DLL(hal)
> WINE_CONFIG_DLL(hhctrl.ocx,,[clean,implib],[htmlhelp])
> WINE_CONFIG_DLL(hid,,[implib])
> +WINE_CONFIG_TEST(dlls/hid/tests)
> WINE_CONFIG_DLL(hidclass.sys,,[implib],[hidclass])
> WINE_CONFIG_DLL(hlink,,[clean,implib])
> WINE_CONFIG_TEST(dlls/hlink/tests)
> diff --git a/dlls/hid/tests/Makefile.in b/dlls/hid/tests/Makefile.in
> new file mode 100644
> index 0000000..d4a69f0
> --- /dev/null
> +++ b/dlls/hid/tests/Makefile.in
> @@ -0,0 +1,5 @@
> +TESTDLL = hid.dll
> +IMPORTS = hid setupapi
> +
> +C_SRCS = \
> + device.c \
The "\" at the end seems wrong. Please note that you can also use tools/make_makefiles
to update/fix this file automatically for you.
> diff --git a/dlls/hid/tests/device.c b/dlls/hid/tests/device.c
> new file mode 100644
> index 0000000..f3260ec
> --- /dev/null
> +++ b/dlls/hid/tests/device.c
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright (c) 2016 Aric Stewart
> + *
> + * 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 "ntstatus.h"
> +#define WIN32_NO_STATUS
> +#include "windows.h"
> +#include "setupapi.h"
> +
> +#ifndef WINE_NTSTATUS_DECLARED
> +#define WINE_NTSTATUS_DECLARED
> +typedef LONG NTSTATUS;
> +#endif
Such hacks should be avoided in tests. I believe you are trying to workaround a
bug in the Wine hidsdi.h header file.
Wine declares NTSTATUS after the includes:
--- snip ---
#include <hidusage.h>
#include <ddk/hidpi.h>
#ifndef WINE_NTSTATUS_DECLARED
#define WINE_NTSTATUS_DECLARED
typedef LONG NTSTATUS;
#endif
--- snip ---
On Windows its declared before:
--- snip ---
typedef _Return_type_success_(return >= 0) LONG NTSTATUS;
#include "hidusage.h"
#include "hidpi.h"
--- snip ---
> +
> +#include "ddk/hidsdi.h"
> +
> +#include "wine/test.h"
> +
> +static void enumerate_devices(void)
> +{
> + GUID hid_guid;
> + WCHAR device_name[128];
> + HDEVINFO info_set;
> + DWORD index = 0;
> + SP_DEVICE_INTERFACE_DATA interface_data;
> + DWORD detail_size = MAX_PATH * sizeof(WCHAR);
> + SP_DEVICE_INTERFACE_DETAIL_DATA_W *data;
> +
> + HidD_GetHidGuid(&hid_guid);
> +
> + ZeroMemory(&interface_data, sizeof(interface_data));
> + interface_data.cbSize = sizeof(interface_data);
> +
> + data = HeapAlloc(GetProcessHeap(), 0, sizeof(*data) + detail_size);
> + data->cbSize = sizeof(*data);
> +
> + info_set = SetupDiGetClassDevsW(&hid_guid, NULL, NULL, DIGCF_DEVICEINTERFACE);
Shouldn't this be released afterwards?
> + while (SetupDiEnumDeviceInterfaces(info_set, NULL, &hid_guid, index, &interface_data))
> + {
> + index ++;
> +
> + if (SetupDiGetDeviceInterfaceDetailW(info_set, &interface_data, data, detail_size, NULL, NULL))
Shouldn't the DeviceInterfaceDetailDataSize parameter also include the sizeof(*data) ?
> + {
> + PHIDP_PREPARSED_DATA ppd;
> + HIDP_CAPS Caps;
> + HANDLE file = CreateFileW(data->DevicePath, GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, 0);
> + if (file == INVALID_HANDLE_VALUE)
> + {
> + trace("Failed to access device %s, likely not plugged in or access is denied.\n", wine_dbgstr_w(data->DevicePath));
> + continue;
> + }
> + ok(HidD_GetPreparsedData(file, &ppd), "Failed to preparsed data\n");
Is there a "get" missing in the debug string?
> + ok(HidP_GetCaps(ppd, &Caps) == HIDP_STATUS_SUCCESS, "Failed to get Caps\n");
> + ok(HidD_GetProductString(file, device_name, sizeof(device_name)), "Failed to get product string\n");
> + trace("Found device %s (%s, %02x, %02x)\n", wine_dbgstr_w(device_name), wine_dbgstr_w(data->DevicePath), Caps.UsagePage, Caps.Usage);
> + ok(HidD_FreePreparsedData(ppd), "Failed to free preparsed data\n");
Its a matter of taste, but usually its better to assign those values to a variable.
This also allows to print the error code / GetLastError() in case of a failure.
> + CloseHandle(file);
> + }
> + }
> + HeapFree(GetProcessHeap(), 0, data);
> +}
> +
> +START_TEST(device)
> +{
> + enumerate_devices();
> +}
>
>
>
More information about the wine-devel
mailing list