Skip to content
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

Using WINUSB_MS_VENDOR_CODE in extra string #7

Merged
merged 1 commit into from
Aug 19, 2019
Merged

Using WINUSB_MS_VENDOR_CODE in extra string #7

merged 1 commit into from
Aug 19, 2019

Conversation

karelbilek
Copy link
Contributor

This might make some future debugging easier.

I did not test this, but it should work in theory :)

Copy link
Owner

@devanlai devanlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that this would work - I think it would create a temporary character array on the stack that would not be guaranteed to still be valid when the extra string descriptor request comes in.

This could be rectified by making it a static const char array.

@karelbilek
Copy link
Contributor Author

You are probably right. I did not test it on our device yet (WinUSB still doesn't load correctly; I think I will have to add the extended properties descriptor).

This might make some future debugging easier.
@karelbilek
Copy link
Contributor Author

karelbilek commented Dec 4, 2017

It's static const now.

@devanlai
Copy link
Owner

devanlai commented Dec 5, 2017

Thanks, when I get a chance to dig out my windows test machine, I'll verify that it still works.

@karelbilek
Copy link
Contributor Author

Seems to be working in our FW

trezor/trezor-mcu#260

@devanlai devanlai merged commit 9bb26fa into devanlai:master Aug 19, 2019
@devanlai
Copy link
Owner

@karel-3d Sorry this took so long to merge - I just finally got around to digging out my Windows 10 test machine. Thanks for the patch!

@karelbilek
Copy link
Contributor Author

karelbilek commented Aug 20, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants