[Bug] Add key event check to is_tap_record and remove is_tap_key
#18063
Conversation
This file contains 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
KarlK90
changed the title
is_tap_record and remove is_tap_keyis_tap_record and remove is_tap_key
...as it is called with tick events and this will generate out of bound access in keymaps.
...and subject to the same bug as is_tap_record.
KarlK90
force-pushed
the
fix/is-tap-record-oob
branch
from
be2e038 to
bcac323
Compare
sigprof
approved these changes
Aug 15, 2022
14 tasks
|
Absolutely, I wonder how many heisenbugs where caused by this. |
|
I ran into some in #15889 and wouldn't have noticed if the tests hadn't caught it as most behaviors were correct 🤷♂️ I expect them doing so saved me quite a bit of time. |
14 tasks
nolanseaton
pushed a commit
to nolanseaton/qmk_firmware
that referenced
this pull request
Jan 23, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
...as it is called with tick events and this would generate out of bound access
(col: 255, row: 255)in keymaps inkeymap_key_to_keycodethough #13619 add a mitigation.. Unit-tests that previously failed are enabled again.is_tap_keyis removed as well because it is not used in the code base and subject to the same bug.Types of Changes
Issues Fixed or Closed by This PR
Checklist