Fix obscure segfault when resizing terminal #248
Merged
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
This was a very obscure segmentation fault I
stumbled across while testing the drive
selection window. I confirmed it exists
in all versions at least going back prior
to 0.24 and maybe a lot earlier.
It was a very tricky bug to track down as you
had to do some very specific things to cause it to
occur. Probably most people would not have seen
it, however for those that did, this was what you
needed to do to trigger it.
1. First you had to be wiping more than one drive.
2. Then you needed to scroll down to the bottom
drive in the list.
3. You then needed to minimise the vertical height
of the terminal so you could no longer see the
selected drive.
4. Then you needed to expand the vertical height
of the terminal. This last step would trigger a
segmentation fault and crash nwipe.
So, the theory. nwipe uses four variables to
allow you to scroll through multiple displayed
drives. These four variables are:
count = the number of enumerated drives.
slots = the number of available horizontal lines
to display the drives. The number of slots
varies depending upon vertical resizing
of the terminal. So slots is calculated
in real time.
focus = which drive the GUI '>' pointer is pointing to.
offset = A value between 0 and slots that describes
what drives are displayed in the available
slots.
offset is then used along with i in a for loop to index
the drive contexts. This is where the segfault
occurred.
The calculation failed to take correct account of a varying
number of slots so in the condition described above the
offset would create an index that was out of bounds.
So first I fixed the miss calculation and second I placed
an if statement that acts as a bounds checker so if
ever this code is changed by someone in the future and
they break the calculation the bounds checker 'if' statement
will log the details of the error and prevent a segfault which
is far easier to debug.
PartialVolume
deleted the
Fix_segfault_when_scrolling_drive_selection_window
branch
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.
This was a very obscure segmentation fault I stumbled across while testing the drive selection window. I confirmed it exists in all versions at least going back prior to 0.24 and maybe a lot earlier.
It was a very tricky bug to track down as you had to do some very specific things to cause it to occur. Probably most people would not have seen it, however for those that did, this was what you needed to do to trigger it.
First you had to be wiping more than one drive.
Then you needed to scroll down to the bottom drive in the list.
You then needed to minimise the vertical height of the terminal so you could no longer see the selected drive.
Then you needed to expand the vertical height of the terminal. This last step would trigger a segmentation fault and crash nwipe.
So, the theory. nwipe uses four variables to allow you to scroll through multiple displayed drives.
These four variables are:
count = the number of enumerated drives.
slots = the number of available horizontal lines to display the drives. The number of slots varies depending upon vertical resizing of the terminal. So slots is calculated in real time.
focus = which drive the GUI '>' pointer is pointing to.
offset = A value between 0 and slots that describes what drives are displayed in the available slots.
offset is then used along with i in a for loop to index the drive contexts. This is where the segfault occurred.
The calculation failed to take correct account of a varying number of slots so in the condition described above the offset would create an index that was out of bounds.
So first I fixed the miss calculation and second I placed an if statement that acts as a bounds checker so if ever this code is changed by someone in the future and they break the calculation the bounds checker 'if' statement will log the details of the error and prevent a segfault which is far easier to debug.