Fix being able to delete locals and WatchWindow::selectedRow becoming negative#220
Merged
Conversation
If there are currently no elements in WatchWindow::rows and WatchWindow::extraRows is equal to 0, WatchLastRow() will return -1. This causes selectedRow to become -1. Since bounds checks only check if selectedRows != rows.Length(), such checks now pass. This in turn causes out bounds accesses. This seems to currently only be a problem when deleting variables from the locals window.
Insert a check when handling the delete key being hit that the active watch window is a normal watch window, not the locals window. Otherwise, the user can delete rows from the locals window.
Owner
|
Thank you for the commit! |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This pull requests fixes #219, which consists of two issues.
First, the user is able to delete rows from the locals window, which seems unintended. To fix this, a check is inserted that ensures that the delete key only deletes rows if the affected window is a normal watch window, not a locals window.
If this feature was intended, I apologize.
Secondly, when deleting rows from the locals window, the index variable
WatchWindow::selectedRowcould become negative. This was due to assigning to it the result ofWatchLastRow(), which could return -1 if bothWatchWindowmembersrows.Length()andextraRowswere equal to 0. As this index was only checked for inequality torows.Length()when deleting, an out of bounds array access was performed.This seems to only have been a problem when able to delete from the locals window, as otherwise
extraRowsis not 0. If the behaviour of deleting from the locals window is intended, this fixes a crash. And even it's not intended, ensuring the variable doesn't go negative is probably a good idea.Thanks for making a great program.