Move search line upper bound check to less generic function
ClosedPublic

Authored by mglb on Jul 14 2018, 12:09 AM.

Details

Summary

When there was at least one search attempt, search box is still opened,
and the screen and/or history is cleared, there is a high chance that
the line number remembered by the search is invalid. The line number is
used as the lines array index, so this can lead to overflow and crash.

This is at this moment fixed with a check in copyLineToStream(), which
is a generic function that happens to be used by search function and
where the line number is used. There still is an assert which is
triggered in debug builds.

The patch moves the check directly to a search function, where the line
number is initialized before first search.

Test Plan

You have to do the test in debug build - there is a hack in
copyLineToStream() which prevents crash, but assert before it
catches an error condition.

  • Run seq 5000
  • Open search box, search for 000
  • Clear screen/history
  • Search up/down

Actual result: Crash
Expected result: Search should begin from last/first visible line

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mglb requested review of this revision.Jul 14 2018, 12:09 AM
mglb created this revision.
mglb updated this revision to Diff 37721.Jul 14 2018, 12:10 AM

Remove qDebugs

Thanks for looking at this. It appears to fix this issue.

unrelated to this bug or patch, I notice 'seq 5000'; hit down arrow, it no longer loops to top of history

hindenburg accepted this revision.Jul 14 2018, 5:09 PM
This revision is now accepted and ready to land.Jul 14 2018, 5:09 PM
This revision was automatically updated to reflect the committed changes.