Skip to content

Commit 8949182

Browse files
author
Stephen
committed
Fixes to report size calculation and added tests
1 parent fe758f9 commit 8949182

File tree

5 files changed

+228
-26
lines changed

5 files changed

+228
-26
lines changed

‎CMakeLists.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ if(HIDAPI_ENABLE_ASAN)
7070
endif()
7171
endif()
7272

73-
if(WIN32)
74-
# so far only Windows has tests
73+
if(WIN32 OR HIDAPI_WITH_LIBUSB)
74+
# so far only Windows and LibUSB have tests
7575
option(HIDAPI_WITH_TESTS "Build HIDAPI (unit-)tests" ${IS_DEBUG_BUILD})
7676
else()
7777
set(HIDAPI_WITH_TESTS OFF)

‎libusb/CMakeLists.txt‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,7 @@ if(HIDAPI_INSTALL_TARGETS)
105105
endif()
106106

107107
hidapi_configure_pc("${PROJECT_ROOT}/pc/hidapi-libusb.pc.in")
108+
109+
if(HIDAPI_WITH_TESTS)
110+
add_subdirectory(test)
111+
endif()

‎libusb/hid.c‎

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -279,27 +279,18 @@ static int get_usage(uint8_t *report_descriptor, size_t size,
279279
return -1; /* failure */
280280
}
281281

282-
/* Retrieves the largest input report size (in bytes) from the report descriptor.
283-
284-
Requires an opened device with *claimed interface*.
285-
282+
/* Retrieves the largest input report size (in bytes) from the passed in report descriptor.
286283
The return value is the size on success and -1 on failure. */
287-
static size_t get_max_input_report_size(libusb_device_handle *handle, int interface_num, uint16_t expected_report_descriptor_size)
284+
static size_t get_max_input_report_size(uint8_t * report_descriptor, int desc_size)
288285
{
289286
int i = 0;
290287
int size_code;
291288
int data_len, key_size;
292289

293-
int64_t report_size = 0, report_count = 0;
290+
int64_t report_size = -1, report_count = -1;
291+
ssize_t cur_size = 0;
294292
ssize_t max_size = -1;
295293

296-
unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE];
297-
298-
int desc_size = hid_get_report_descriptor_libusb(handle, interface_num, expected_report_descriptor_size, report_descriptor, sizeof(report_descriptor));
299-
if (desc_size < 0) {
300-
return -1;
301-
}
302-
303294
while (i < desc_size) {
304295
int key = report_descriptor[i];
305296
int key_cmd = key & 0xfc;
@@ -325,26 +316,41 @@ static size_t get_max_input_report_size(libusb_device_handle *handle, int interf
325316
key_size = 1;
326317
}
327318

328-
if (key_cmd == 0x94) {
319+
if (key_cmd == 0x94) { /* Report Count */
329320
report_count = get_bytes(report_descriptor, desc_size, data_len, i);
330321
}
331-
if (key_cmd == 0x74) {
322+
if (key_cmd == 0x74) { /* Report Size */
332323
report_size = get_bytes(report_descriptor, desc_size, data_len, i);
333324
}
334-
if (key_cmd == 0x80) { // Input
335-
/* report_size is in bits. Determine the total size (count * size),
336-
convert to bytes (rounded up), and add one byte for the report
337-
number. */
338-
ssize_t size = (((report_count * report_size) + 7) / 8) + 1;
339-
if (size > max_size)
340-
max_size = size;
325+
if (key_cmd == 0x80) { /* Input */
326+
if (report_count < 0 || report_size < 0) {
327+
/* We are missing size or count. That isn't good. */
328+
return 0;
329+
}
330+
cur_size += (report_count * report_size);
331+
}
332+
if (key_cmd == 0x84) { /* Report ID */
333+
if (cur_size > max_size) {
334+
max_size = cur_size;
335+
}
336+
cur_size = 0;
341337
}
342338

343339
/* Skip over this key and it's associated data */
344340
i += data_len + key_size;
345341
}
346342

347-
return max_size;
343+
if (cur_size > max_size) {
344+
max_size = cur_size;
345+
}
346+
347+
if (max_size < 0) {
348+
return -1;
349+
}
350+
351+
/* report_size is in bits. Determine the total size convert to bytes
352+
(rounded up), and add one byte for the report number. */
353+
return ((max_size + 7) / 8) + 1;
348354
}
349355

350356
#if defined(__FreeBSD__) && __FreeBSD__ < 10
@@ -1294,7 +1300,15 @@ static int hidapi_initialize_device(hid_device *dev, const struct libusb_interfa
12941300
dev->interface = intf_desc->bInterfaceNumber;
12951301

12961302
dev->report_descriptor_size = get_report_descriptor_size_from_interface_descriptors(intf_desc);
1297-
dev->max_input_report_size = get_max_input_report_size(dev->device_handle, dev->interface, dev->report_descriptor_size);
1303+
1304+
unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE];
1305+
1306+
int desc_size = hid_get_report_descriptor_libusb(dev->device_handle, dev->interface, dev->report_descriptor_size, report_descriptor, sizeof(report_descriptor));
1307+
if (desc_size > 0) {
1308+
dev->max_input_report_size = get_max_input_report_size(report_descriptor, desc_size);
1309+
} else {
1310+
dev->max_input_report_size = -1;
1311+
}
12981312

12991313
dev->input_endpoint = 0;
13001314
dev->input_ep_max_packet_size = 0;

‎libusb/test/CMakeLists.txt‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
add_executable(max_input_report_size_test max_input_report_size_test.c)
2+
set_target_properties(max_input_report_size_test
3+
PROPERTIES
4+
C_STANDARD 11
5+
C_STANDARD_REQUIRED TRUE
6+
)
7+
8+
if(TARGET usb-1.0)
9+
target_link_libraries(max_input_report_size_test PRIVATE usb-1.0)
10+
else()
11+
include(FindPkgConfig)
12+
pkg_check_modules(libusb REQUIRED IMPORTED_TARGET libusb-1.0>=1.0.9)
13+
target_link_libraries(max_input_report_size_test PRIVATE PkgConfig::libusb)
14+
endif()
15+
16+
target_link_libraries(max_input_report_size_test PUBLIC hidapi_include)
17+
18+
# Each test case requires 2 files:
19+
# <name>.pp_data - textual representation of HIDP_PREPARSED_DATA;
20+
# <name>_expected.rpt_desc - reconstructed HID Report Descriptor out of .pp_data file;
21+
#
22+
# (Non-required by test):
23+
# <name>_real.dpt_desc - the original report rescriptor used to create a test case;
24+
set(HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES
25+
046D_C52F_0001_000C
26+
046D_C52F_0001_FF00
27+
046D_C52F_0002_0001
28+
046D_C52F_0002_FF00
29+
17CC_1130_0000_FF01
30+
046D_0A37_0001_000C
31+
046A_0011_0006_0001
32+
046D_C077_0002_0001
33+
046D_C283_0004_0001
34+
046D_B010_0006_0001
35+
046D_B010_0002_FF00
36+
046D_B010_0002_0001
37+
046D_B010_0001_FF00
38+
046D_B010_0001_000C
39+
046D_C534_0001_000C
40+
046D_C534_0001_FF00
41+
046D_C534_0002_0001
42+
046D_C534_0002_FF00
43+
046D_C534_0006_0001
44+
046D_C534_0080_0001
45+
047F_C056_0001_000C
46+
047F_C056_0003_FFA0
47+
047F_C056_0005_000B
48+
045E_02FF_0005_0001
49+
1532_00A3_0002_0001
50+
)
51+
52+
set(CMAKE_VERSION_SUPPORTS_ENVIRONMENT_MODIFICATION "3.22")
53+
54+
foreach(TEST_CASE ${HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES})
55+
set(TEST_PP_DATA "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}.pp_data")
56+
if(NOT EXISTS "${TEST_PP_DATA}")
57+
message(FATAL_ERROR "Missing '${TEST_PP_DATA}' file for '${TEST_CASE}' test case")
58+
endif()
59+
set(TEST_EXPECTED_DESCRIPTOR "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}_expected.rpt_desc")
60+
if(NOT EXISTS "${TEST_EXPECTED_DESCRIPTOR}")
61+
message(FATAL_ERROR "Missing '${TEST_EXPECTED_DESCRIPTOR}' file for '${TEST_CASE}' test case")
62+
endif()
63+
64+
add_test(NAME "LibUsbHidMaxInputReportSizeTest_${TEST_CASE}"
65+
COMMAND max_input_report_size_test "${TEST_PP_DATA}" "${TEST_EXPECTED_DESCRIPTOR}"
66+
WORKING_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}"
67+
#WORKING_DIRECTORY "$<TARGET_FILE_DIR:hidapi_winapi>"
68+
)
69+
endforeach()

‎libusb/test/max_input_report_size_test.c‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
#include <stddef.h>
2+
#include <stdio.h>
3+
#include <stdlib.h>
4+
#include <stdbool.h>
5+
#include <string.h>
6+
#include <errno.h>
7+
8+
#include "../hid.c"
9+
10+
static ssize_t parse_max_input_report_size(const char * filename)
11+
{
12+
FILE* file = fopen(filename, "r");
13+
if (file == NULL) {
14+
fprintf(stderr, "ERROR: Couldn't open file '%s' for reading: %s\n", filename, strerror(errno));
15+
return -1;
16+
}
17+
18+
char line[256];
19+
{
20+
while (fgets(line, sizeof(line), file) != NULL) {
21+
unsigned short temp_ushort;
22+
if (sscanf(line, "pp_data->caps_info[0]->ReportByteLength = %hu\n", &temp_ushort) == 1) {
23+
fclose(file);
24+
return (ssize_t)temp_ushort;
25+
}
26+
}
27+
}
28+
29+
fprintf(stderr, "Unable to find pp_data->caps_info[0]->ReportByteLength in %s\n", filename);
30+
fclose(file);
31+
32+
return -1;
33+
}
34+
35+
static bool read_hex_data_from_text_file(const char *filename, unsigned char *data_out, size_t data_size, size_t *actual_read)
36+
{
37+
size_t read_index = 0;
38+
FILE* file = fopen(filename, "r");
39+
if (file == NULL) {
40+
fprintf(stderr, "ERROR: Couldn't open file '%s' for reading: %s\n", filename, strerror(errno));
41+
return false;
42+
}
43+
44+
bool result = true;
45+
unsigned int val;
46+
char buf[16];
47+
while (fscanf(file, "%15s", buf) == 1) {
48+
if (sscanf(buf, "0x%X", &val) != 1) {
49+
fprintf(stderr, "Invalid HEX text ('%s') file, got %s\n", filename, buf);
50+
result = false;
51+
goto end;
52+
}
53+
54+
if (read_index >= data_size) {
55+
fprintf(stderr, "Buffer for file read is too small. Got only %zu bytes to read '%s'\n", data_size, filename);
56+
result = false;
57+
goto end;
58+
}
59+
60+
if (val > (unsigned char)-1) {
61+
fprintf(stderr, "Invalid HEX text ('%s') file, got a value of: %u\n", filename, val);
62+
result = false;
63+
goto end;
64+
}
65+
66+
data_out[read_index] = (unsigned char) val;
67+
68+
read_index++;
69+
}
70+
71+
if (!feof(file)) {
72+
fprintf(stderr, "Invalid HEX text ('%s') file - failed to read all values\n", filename);
73+
result = false;
74+
goto end;
75+
}
76+
77+
*actual_read = read_index;
78+
79+
end:
80+
fclose(file);
81+
return result;
82+
}
83+
84+
85+
int main(int argc, char* argv[])
86+
{
87+
if (argc != 3) {
88+
fprintf(stderr, "Expected 2 arguments for the test ('<>.pp_data' and '<>_expected.rpt_desc'), got: %d\n", argc - 1);
89+
return EXIT_FAILURE;
90+
}
91+
92+
printf("Checking: '%s' / '%s'\n", argv[1], argv[2]);
93+
94+
unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE];
95+
size_t report_descriptor_size = 0;
96+
if (!read_hex_data_from_text_file(argv[2], report_descriptor, sizeof(report_descriptor), &report_descriptor_size)) {
97+
return EXIT_FAILURE;
98+
}
99+
100+
ssize_t expected = parse_max_input_report_size(argv[1]);
101+
if (expected < 0) {
102+
fprintf(stderr, "Unable to expected max input report size from %s\n", argv[1]);
103+
return EXIT_FAILURE;
104+
}
105+
106+
ssize_t res = (ssize_t)get_max_input_report_size(report_descriptor, report_descriptor_size);
107+
108+
if (res != expected) {
109+
fprintf(stderr, "Failed to properly compute size. Got %zd, expected %zd\n", res, expected);
110+
return EXIT_FAILURE;
111+
} else {
112+
printf("Properly computed size: %zd\n", res);
113+
return EXIT_SUCCESS;
114+
}
115+
}

0 commit comments

Comments
 (0)