-
Notifications
You must be signed in to change notification settings - Fork 477
Properly determine libusb read size for large reports (Fixes #274) #728
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
sudobash1
wants to merge
16
commits into
libusb:master
Choose a base branch
from
sudobash1:libusb_input_size
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
76b72c6
Properly determine libusb read size for large reports
09017e5
Fixes to report size calculation and added tests
415db44
Allow computation of max report size for other types
8b07bcf
Don't assign -1 to size_t. That's silly
3b3c70a
Do not increase max_report_size by one if not using report numbers
c358d33
Fix compiler warnings
e19ba07
Remove comment
sudobash1 6eedce1
Merge branch 'master' into libusb_input_size
mcuee cda57eb
Merge branch 'master' into libusb_input_size
mcuee 7ebb679
Merge branch 'master' into libusb_input_size
mcuee 1ab6b3f
Merge branch 'master' into libusb_input_size
mcuee dffba86
Merge branch 'master' into libusb_input_size
mcuee 39e58ae
Address libusb report-size review feedback
Youw 62c717d
Merge master into libusb_input_size
Youw 8729fcb
Harden libusb report size handling
Youw 580996e
Merge master into libusb_input_size
Youw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| add_executable(max_input_report_size_test max_input_report_size_test.c) | ||
| set_target_properties(max_input_report_size_test | ||
| PROPERTIES | ||
| C_STANDARD 11 | ||
| C_STANDARD_REQUIRED TRUE | ||
| ) | ||
|
|
||
| if(TARGET usb-1.0) | ||
| target_link_libraries(max_input_report_size_test PRIVATE usb-1.0) | ||
| else() | ||
| include(FindPkgConfig) | ||
| pkg_check_modules(libusb REQUIRED IMPORTED_TARGET libusb-1.0>=1.0.9) | ||
| target_link_libraries(max_input_report_size_test PRIVATE PkgConfig::libusb) | ||
| endif() | ||
|
|
||
| target_link_libraries(max_input_report_size_test PUBLIC hidapi_include) | ||
|
|
||
| # Each test case requires 2 files: | ||
| # <name>.pp_data - textual representation of HIDP_PREPARSED_DATA; | ||
| # <name>_expected.rpt_desc - reconstructed HID Report Descriptor out of .pp_data file; | ||
| # | ||
| # (Non-required by test): | ||
| # <name>_real.dpt_desc - the original report rescriptor used to create a test case; | ||
|
Youw marked this conversation as resolved.
Outdated
|
||
| set(HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES | ||
| 046D_C52F_0001_000C | ||
| 046D_C52F_0001_FF00 | ||
| 046D_C52F_0002_0001 | ||
| 046D_C52F_0002_FF00 | ||
| 17CC_1130_0000_FF01 | ||
| 046D_0A37_0001_000C | ||
| 046A_0011_0006_0001 | ||
| 046D_C077_0002_0001 | ||
| 046D_C283_0004_0001 | ||
| 046D_B010_0006_0001 | ||
| 046D_B010_0002_FF00 | ||
| 046D_B010_0002_0001 | ||
| 046D_B010_0001_FF00 | ||
| 046D_B010_0001_000C | ||
| 046D_C534_0001_000C | ||
| 046D_C534_0001_FF00 | ||
| 046D_C534_0002_0001 | ||
| 046D_C534_0002_FF00 | ||
| 046D_C534_0006_0001 | ||
| 046D_C534_0080_0001 | ||
| 047F_C056_0001_000C | ||
| 047F_C056_0003_FFA0 | ||
| 047F_C056_0005_000B | ||
| 045E_02FF_0005_0001 | ||
| 1532_00A3_0002_0001 | ||
| ) | ||
|
|
||
| set(CMAKE_VERSION_SUPPORTS_ENVIRONMENT_MODIFICATION "3.22") | ||
|
|
||
| foreach(TEST_CASE ${HID_DESCRIPTOR_RECONSTRUCT_TEST_CASES}) | ||
| set(TEST_PP_DATA "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}.pp_data") | ||
|
Youw marked this conversation as resolved.
|
||
| if(NOT EXISTS "${TEST_PP_DATA}") | ||
| message(FATAL_ERROR "Missing '${TEST_PP_DATA}' file for '${TEST_CASE}' test case") | ||
| endif() | ||
| set(TEST_EXPECTED_DESCRIPTOR "${CMAKE_CURRENT_LIST_DIR}/../../windows/test/data/${TEST_CASE}_expected.rpt_desc") | ||
|
Youw marked this conversation as resolved.
Outdated
|
||
| if(NOT EXISTS "${TEST_EXPECTED_DESCRIPTOR}") | ||
| message(FATAL_ERROR "Missing '${TEST_EXPECTED_DESCRIPTOR}' file for '${TEST_CASE}' test case") | ||
| endif() | ||
|
|
||
| add_test(NAME "LibUsbHidMaxInputReportSizeTest_${TEST_CASE}" | ||
| COMMAND max_input_report_size_test "${TEST_PP_DATA}" "${TEST_EXPECTED_DESCRIPTOR}" | ||
| WORKING_DIRECTORY "${CMAKE_CURRENT_LIST_DIR}" | ||
| ) | ||
| endforeach() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,164 @@ | ||
| #include <stddef.h> | ||
| #include <stdio.h> | ||
| #include <stdlib.h> | ||
| #include <stdbool.h> | ||
| #include <string.h> | ||
| #include <errno.h> | ||
|
|
||
| #include "../hid.c" | ||
|
|
||
| struct max_report_sizes { | ||
| size_t input; | ||
| size_t output; | ||
| size_t feature; | ||
| }; | ||
|
|
||
| static int parse_max_input_report_size(const char * filename, struct max_report_sizes * sizes) | ||
| { | ||
| FILE* file = fopen(filename, "r"); | ||
| if (file == NULL) { | ||
| fprintf(stderr, "ERROR: Couldn't open file '%s' for reading: %s\n", filename, strerror(errno)); | ||
| return -1; | ||
| } | ||
|
|
||
| int has_report_id = 0; | ||
|
|
||
| char line[256]; | ||
| { | ||
| while (fgets(line, sizeof(line), file) != NULL) { | ||
| unsigned short temp_ushort; | ||
| if (sscanf(line, "pp_data->caps_info[0]->ReportByteLength = %hu\n", &temp_ushort) == 1) { | ||
| sizes->input = (size_t)temp_ushort; | ||
| } | ||
| if (sscanf(line, "pp_data->caps_info[1]->ReportByteLength = %hu\n", &temp_ushort) == 1) { | ||
| sizes->output = (size_t)temp_ushort; | ||
| } | ||
| if (sscanf(line, "pp_data->caps_info[2]->ReportByteLength = %hu\n", &temp_ushort) == 1) { | ||
| sizes->feature = (size_t)temp_ushort; | ||
| } | ||
| if (sscanf(line, "pp_data->cap[%*u]->ReportID = 0x%hu\n", &temp_ushort) == 1) { | ||
| if (temp_ushort) { | ||
| has_report_id = 1; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| fclose(file); | ||
|
|
||
| // Windows includes ReportID byte in descriptor size even when it is not | ||
| // used. Our libusb calculation does not include it, so remove one byte | ||
| // from the sizes to make it match. | ||
| if (!has_report_id) { | ||
| if (sizes->input) { | ||
| sizes->input--; | ||
| } | ||
| if (sizes->output) { | ||
| sizes->output--; | ||
| } | ||
| if (sizes->feature) { | ||
| sizes->feature--; | ||
| } | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| static bool read_hex_data_from_text_file(const char *filename, unsigned char *data_out, size_t data_size, size_t *actual_read) | ||
| { | ||
| size_t read_index = 0; | ||
| FILE* file = fopen(filename, "r"); | ||
| if (file == NULL) { | ||
| fprintf(stderr, "ERROR: Couldn't open file '%s' for reading: %s\n", filename, strerror(errno)); | ||
| return false; | ||
| } | ||
|
|
||
| bool result = true; | ||
| unsigned int val; | ||
| char buf[16]; | ||
| while (fscanf(file, "%15s", buf) == 1) { | ||
| if (sscanf(buf, "0x%X", &val) != 1) { | ||
| fprintf(stderr, "Invalid HEX text ('%s') file, got %s\n", filename, buf); | ||
| result = false; | ||
| goto end; | ||
| } | ||
|
|
||
| if (read_index >= data_size) { | ||
| fprintf(stderr, "Buffer for file read is too small. Got only %zu bytes to read '%s'\n", data_size, filename); | ||
| result = false; | ||
| goto end; | ||
| } | ||
|
|
||
| if (val > (unsigned char)-1) { | ||
| fprintf(stderr, "Invalid HEX text ('%s') file, got a value of: %u\n", filename, val); | ||
| result = false; | ||
| goto end; | ||
| } | ||
|
|
||
| data_out[read_index] = (unsigned char) val; | ||
|
|
||
| read_index++; | ||
| } | ||
|
|
||
| if (!feof(file)) { | ||
| fprintf(stderr, "Invalid HEX text ('%s') file - failed to read all values\n", filename); | ||
| result = false; | ||
| goto end; | ||
| } | ||
|
|
||
| *actual_read = read_index; | ||
|
|
||
| end: | ||
| fclose(file); | ||
| return result; | ||
| } | ||
|
|
||
|
|
||
| int main(int argc, char* argv[]) | ||
| { | ||
| if (argc != 3) { | ||
| fprintf(stderr, "Expected 2 arguments for the test ('<>.pp_data' and '<>_expected.rpt_desc'), got: %d\n", argc - 1); | ||
| return EXIT_FAILURE; | ||
| } | ||
|
|
||
| printf("Checking: '%s' / '%s'\n", argv[1], argv[2]); | ||
|
|
||
| unsigned char report_descriptor[HID_API_MAX_REPORT_DESCRIPTOR_SIZE]; | ||
| size_t report_descriptor_size = 0; | ||
| if (!read_hex_data_from_text_file(argv[2], report_descriptor, sizeof(report_descriptor), &report_descriptor_size)) { | ||
| return EXIT_FAILURE; | ||
| } | ||
|
|
||
| struct max_report_sizes expected; | ||
| if (parse_max_input_report_size(argv[1], &expected) < 0) { | ||
| fprintf(stderr, "Unable to get expected max report sizes from %s\n", argv[1]); | ||
| return EXIT_FAILURE; | ||
| } | ||
|
|
||
| struct max_report_sizes computed = { | ||
| .input = (size_t)get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_INPUT), | ||
| .output = (size_t)get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_OUTPUT), | ||
| .feature = (size_t)get_max_report_size(report_descriptor, report_descriptor_size, REPORT_DESCR_FEATURE) | ||
| }; | ||
|
|
||
| int ret = EXIT_SUCCESS; | ||
|
|
||
| if (expected.input != computed.input) { | ||
| fprintf(stderr, "Failed to properly compute input size. Got %zu, expected %zu\n", computed.input, expected.input); | ||
| ret = EXIT_FAILURE; | ||
| } | ||
| if (expected.output != computed.output) { | ||
| fprintf(stderr, "Failed to properly compute output size. Got %zu, expected %zu\n", computed.output, expected.output); | ||
| ret = EXIT_FAILURE; | ||
| } | ||
| if (expected.feature != computed.feature) { | ||
| fprintf(stderr, "Failed to properly compute feature size. Got %zu, expected %zu\n", computed.feature, expected.feature); | ||
| ret = EXIT_FAILURE; | ||
| } | ||
|
|
||
| if (ret == EXIT_SUCCESS) { | ||
| printf("Properly computed sizes: %zu, %zu, %zu\n", computed.input, computed.output, computed.feature); | ||
| } | ||
|
|
||
| return ret; | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.