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

Fix argument to ReadConsoleW #62

Merged
merged 1 commit into from
Dec 3, 2022
Merged

Fix argument to ReadConsoleW #62

merged 1 commit into from
Dec 3, 2022

Conversation

mjunix
Copy link
Contributor

@mjunix mjunix commented Dec 3, 2022

(I have no experience with this code base so someone should probably review this before merging.)

Copy link
Owner

@LekKit LekKit left a comment

Choose a reason for hiding this comment

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

Yeah I approve this, thanks. Somehow I missed that ReadConsoleW takes number of characters instead of buffer size, and the lack of continuous Win32 testing let it like that (Tho it's weird that none of our static analyzers warned on it).
All builds (MinGW, MSVC, mingwCE) passed. Will test on a Windows VM soon before merging.

I assume you caught this by copy-pasting a lot of text sorta?

@LekKit
Copy link
Owner

LekKit commented Dec 3, 2022

Tested that VT works properly on Win32, no problems with large pasted inputs.
It seems that static analyzers missed it because the project is mostly developed/tested/analyzed on Linux, whether Win32-specific code doesn't receive that much attention. This is a sign this needs improvement in future.

image

@LekKit LekKit merged commit 4041c0b into LekKit:staging Dec 3, 2022
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