Fix program crash on "m" with large value of "current"#10
Fix program crash on "m" with large value of "current"#10stesser wants to merge 3 commits intochadmiller:mainfrom
Conversation
Fix program crash if the "m" command is used and "current" is higher than supported by the current stats array length.
Reset stats_history[] when switching the transform function - AFAICT the contents of the stats_history array depends on the transformation performed on the gathered statistics data.
Improve response time of "m" command to switch between device centric and measurement centric views
|
I don't have a bug report so I don't know what it's fixing yet. The stats array can change? As in, the device inventory can change, or the dimensions of measurement can change? |
|
The value of "current" is limited to (0 .. length(stats)-1) at the end of the loop. But when "m" is used to switch the mode, "current" may be too large for the array. The pull request consists of 3 parts:
I had missed the fact that current must be within bounds of an array that changes size depending on the device vs. measurements display mode when submitting my previous pull request. These changes make the use of "m" safe and immediately resulting in the selected display mode. |
|
Ah. |
| should_show_differential = not should_show_differential | ||
| elif in_key == ord('m'): | ||
| stats = None | ||
| stats_history = [] |
There was a problem hiding this comment.
Instead of discarding recent history, I'd prefer it if stats and stats_history were unaffected by views and transforming functions were only used to interpret the raw data.
There was a problem hiding this comment.
If you do not want to clear the stats_history array, I'd assume that you need two arrays, one for the device centric view, the other for the measurements centric view.
Always applying both transforms and just selecting one of 2 stats and stats_history arrays for display would be a simple change, but my main intend was to provide a fix for the issue that resulted from the addition of the "m" command.
If you provide 2 sets of arrays and put the restriction of "current" to the allowed index range at the start of the loop, then resetting of current to 0 could also be dropped.
I'll update the FreeBSD port to whatever you decide to implement.
Fix program crash if the "m" command is used and "current" is higher than supported by the current stats array length.
The "m" command has been part of a previous pull request that has been merged. I forgot that the stats array length can change and that the stats_history contents depends on the transform function selected.
This patch fixes this issue. The value of "current" is reset to one that is valid for all possible stats array lengths, and stats_history is reset to an empty array to prevent incompatible data to be used in a difference calculation.