Skip to content

Commit 1edc59d

Browse files
committed
BugFix: try and prevent crashing during highlighting
It seems that one of the tests in `(void) TTextEdit::highlightSelection()` was being done too late - with the result that out of range indexing was being attempted on the `(std::deque<std::deque<TChar>) TBuffer::buffer` member. Specifically the test for `currentY >= totaly` should have been done before `currentY` was used to select one from the outer `std::deque` container so as to establish the size of it (to be stored as `maxX`). The revised code replaces both the highlight and un-highlight code nested loops with components that are more conventionally used, i.e. with the test being based on a less than condition only with the limit being determined within the top parts of the for loops and no additional tests to break from the loop within the body thereof... Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
1 parent f4afc37 commit 1edc59d

File tree

1 file changed

+34
-34
lines changed

1 file changed

+34
-34
lines changed

src/TTextEdit.cpp

Lines changed: 34 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -645,28 +645,27 @@ void TTextEdit::highlightSelection()
645645

646646
mSelectedRegion = mSelectedRegion.subtracted(newRegion);
647647

648-
int startY = mPA.y();
649-
auto totalY = static_cast<int>(mpBuffer->buffer.size());
650-
for (int currentY = startY, total = mPB.y(); currentY <= total; ++currentY) {
651-
int currentX = 0;
652-
int maxX = static_cast<int>(mpBuffer->buffer.at(currentY).size());
653-
if (currentY == startY) {
654-
currentX = mPA.x();
655-
}
656-
657-
if (currentY >= totalY) {
658-
break;
659-
}
660-
661-
for (; currentX < maxX; ++currentX) {
662-
if ((currentY == mPB.y()) && (currentX > mPB.x())) {
663-
break;
664-
}
665-
if (!(mpBuffer->buffer.at(currentY).at(currentX).isSelected())) {
666-
mpBuffer->buffer.at(currentY).at(currentX).select();
667-
}
648+
// mpA represents the first (zero-based) line/row (y) and position/column
649+
// (x) that IS to be selected, mpB represents the last (zero-based) line and
650+
// column that IS to be selected - which means that in the traditional 'for'
651+
// loop construct where the test is a '<' based one, the test limit is
652+
// mpB.y() + 1 for the row and mpB.x() + 1 for the column:
653+
// clang-format off
654+
for (int y = std::max(0, mPA.y()), endY = std::min((mPB.y() + 1), static_cast<int>(mpBuffer->buffer.size()));
655+
y < endY;
656+
++y) {
657+
658+
for (int x = (y == mPA.y()) ? std::max(0, mPA.x()) : 0,
659+
endX = (y == (mPB.y()))
660+
? std::min((mPB.x() + 1), static_cast<int>(mpBuffer->buffer.at(y).size()))
661+
: static_cast<int>(mpBuffer->buffer.at(y).size());
662+
x < endX;
663+
++x) {
664+
665+
mpBuffer->buffer.at(y).at(x).select();
668666
}
669667
}
668+
// clang-format on
670669

671670
update(mSelectedRegion.boundingRect());
672671
update(newRegion);
@@ -676,23 +675,24 @@ void TTextEdit::highlightSelection()
676675
void TTextEdit::unHighlight()
677676
{
678677
normaliseSelection();
679-
int y1 = mPA.y();
680678

681-
if (y1 < 0) {
682-
return;
683-
}
684-
for (int y = y1, total = mPB.y(); y <= total; ++y) {
685-
int x = 0;
686-
687-
if (y >= static_cast<int>(mpBuffer->buffer.size())) {
688-
break;
679+
// clang-format off
680+
for (int y = std::max(0, mPA.y()), endY = std::min((mPB.y() + 1), static_cast<int>(mpBuffer->buffer.size()));
681+
y < endY;
682+
++y) {
683+
684+
for (int x = (y == mPA.y()) ? std::max(0, mPA.x()) : 0,
685+
endX = (y == (mPB.y()))
686+
? std::min((mPB.x() + 1), static_cast<int>(mpBuffer->buffer.at(y).size()))
687+
: static_cast<int>(mpBuffer->buffer.at(y).size());
688+
x < endX;
689+
++x) {
690+
691+
mpBuffer->buffer.at(y).at(x).deselect();
689692
}
690-
691-
for (; x < static_cast<int>(mpBuffer->buffer.at(y).size()); ++x)
692-
if (mpBuffer->buffer.at(y).at(x).isSelected()) {
693-
mpBuffer->buffer.at(y).at(x).deselect();
694-
}
695693
}
694+
// clang-format on
695+
696696
forceUpdate();
697697
}
698698

0 commit comments

Comments
 (0)