mirrors/foot
1
0
Fork 0
mirror of https://codeberg.org/dnkl/foot.git synced 2026-02-05 04:06:08 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

search: fix multiple crashes

Browse source 
* When extending the selection to the next word boundary, ensure the
  row numbers are valid:

  - use selection_get_end() when retrieving the current end
    coordinate. This alone fixes a crash where we previously would
    crash in an out-of-bounds array access in grid->rows[], due to
    term->selection.coords.end being unbounded.

  - ensure the new end coordinate is bounded before and after calling
    selection_find_word_boundary_right().

* When moving the viewport (to ensure a new match is visible), make
  sure we don’t end up with the match outside the new viewport.

  Under certain, unusual, circumstances, the moved viewport _still_
  did not contain the match. This resulted in assertions triggering
  later, that assumed the match(es) are *always* visible.

  It’s fairly easy to trigger this one by running foot with e.g.

    foot -o scrollback.lines=0 --window-size-chars 25x3

  and then hitting enter a couple of times, to fill the scrollback
  history (which should consist of a single row in the example above),
  and the do a scrollback search for (part of) the prompt, and keep
  searching backward until it crashes.

  This would happen if calculated (new) viewport had to be adjusted
  (for example, to ensure it didn’t go past the scrollback end).

  This patch changes the logic used when calculating the new
  viewport. Instead of calculating the wanted viewport (match is
  vertically centered) and then trying to adjust it to ensure the new
  viewport is valid, start with a “safe” new viewport value, and then
  determine how much we can move it, if at all, to center the match.

  This is done by using scrollback relative coordinates. In this
  coordinate system, the new viewport must be

    >= 0, and < grid->num_lines - term->rows

  This makes it very easy to limit the amount by which the viewport is
  adjusted.

  As a side-effect, we can remove all the old re-adjustment logic.

* The match iterator no longer special cases the primary match. This
  was needed before, when the search iterator did not handle
  overlapping matches correctly. Now that we do, the iterator is
  guaranteed to find the primary match, and thus we no longer need to
  special case it.

  This fixes a bug where the primary match was returned twice, due to
  the logic checking if a secondary match is the same as the primary
  match was flawed...

Closes #1036
This commit is contained in:
Daniel Eklöf 2022-04-25 20:00:47 +02:00
parent 1d4e1b921d
commit 5c4ddebc3c
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
1 changed files with 76 additions and 118 deletions
Download patch file Download diff file Expand all files Collapse all files

194
search.c
View file

@ -82,11 +82,7 @@ search_ensure_size(struct terminal *term, size_t wanted_size)
static bool
has_wrapped_around(const struct terminal *term, int abs_row_no)
{
const struct grid *grid = term->grid;
int scrollback_start = grid->offset + term->rows;
int rebased_row = abs_row_no - scrollback_start + grid->num_rows;
rebased_row &= grid->num_rows - 1;
int rebased_row = grid_row_abs_to_sb(term->grid, term->rows, abs_row_no);
return rebased_row == 0;
}
@ -182,6 +178,9 @@ search_update_selection(struct terminal *term, const struct range *match)
int end_row = match->end.row;
int end_col = match->end.col;
xassert(start_row >= 0);
xassert(start_row < grid->num_rows);
bool move_viewport = true;
int view_end = (grid->view + term->rows - 1) & (grid->num_rows - 1);
@ -196,24 +195,14 @@ search_update_selection(struct terminal *term, const struct range *match)
}
if (move_viewport) {
int old_view = grid->view;
int new_view = start_row - term->rows / 2;
int rebased_new_view = grid_row_abs_to_sb(grid, term->rows, start_row);
while (new_view < 0)
new_view += grid->num_rows;
rebased_new_view -= term->rows / 2;
rebased_new_view =
min(max(rebased_new_view, 0), grid->num_rows - term->rows);
new_view = ensure_view_is_allocated(term, new_view);
/* Don't scroll past scrollback history */
int end = (grid->offset + term->rows - 1) & (grid->num_rows - 1);
if (end >= grid->offset) {
/* Not wrapped */
if (new_view >= grid->offset && new_view <= end)
new_view = grid->offset;
} else {
if (new_view >= grid->offset || new_view <= end)
new_view = grid->offset;
}
const int old_view = grid->view;
int new_view = grid_row_sb_to_abs(grid, term->rows, rebased_new_view);
#if defined(_DEBUG)
/* Verify all to-be-visible rows have been allocated */
@ -227,23 +216,8 @@ search_update_selection(struct terminal *term, const struct range *match)
term_damage_view(term);
}
#if 0
/* Selection endpoint is inclusive */
if (--end_col < 0) {
end_col = term->cols - 1;
end_row--;
}
#endif
/*
* Begin a new selection if the start coords changed
*
* Note: check column against selection.coords, since our “old”
* start column isn’t reliable - we modify it when searching
* “next” or “prev”.
*/
if (start_row != term->search.match.row ||
start_col != term->selection.coords.start.col)
start_col != term->search.match.col)
{
int selection_row = start_row - grid->view + grid->num_rows;
selection_row &= grid->num_rows - 1;
@ -516,7 +490,7 @@ search_matches_new_iter(struct terminal *term)
{
return (struct search_match_iterator){
.term = term,
.start = {-2, -2},
.start = {0, 0},
};
}
@ -529,86 +503,63 @@ search_matches_next(struct search_match_iterator *iter)
if (term->search.match_len == 0)
goto no_match;
struct range match;
bool found;
const bool return_primary_match =
iter->start.row == -2 && term->selection.coords.end.row >= 0;
if (return_primary_match) {
/* First, return the primary match */
match = term->selection.coords;
found = true;
}
else if (iter->start.row >= term->rows) {
if (iter->start.row >= term->rows)
goto no_match;
xassert(iter->start.row >= 0);
xassert(iter->start.row < term->rows);
xassert(iter->start.col >= 0);
xassert(iter->start.col < term->cols);
struct coord abs_start = iter->start;
abs_start.row = grid_row_absolute_in_view(grid, abs_start.row);
struct coord abs_end = {
term->cols - 1,
grid_row_absolute_in_view(grid, term->rows - 1)};
struct range match;
bool found = find_next(term, SEARCH_FORWARD, abs_start, abs_end, &match);
if (!found)
goto no_match;
LOG_DBG("match at (absolute coordinates) %dx%d-%dx%d",
match.start.row, match.start.col,
match.end.row, match.end.col);
/* Convert absolute row numbers back to view relative */
match.start.row = match.start.row - grid->view + grid->num_rows;
match.start.row &= grid->num_rows - 1;
match.end.row = match.end.row - grid->view + grid->num_rows;
match.end.row &= grid->num_rows - 1;
LOG_DBG("match at (view-local coordinates) %dx%d-%dx%d, view=%d",
match.start.row, match.start.col,
match.end.row, match.end.col, grid->view);
xassert(match.start.row >= 0);
xassert(match.start.row < term->rows);
xassert(match.end.row >= 0);
xassert(match.end.row < term->rows);
xassert(match.end.row > match.start.row ||
(match.end.row == match.start.row &&
match.end.col >= match.start.col));
/* Continue at next column, next time */
iter->start.row = match.start.row;
iter->start.col = match.start.col + 1;
if (iter->start.col >= term->cols) {
iter->start.col = 0;
iter->start.row++; /* Overflow is caught in next iteration */
}
else {
xassert(iter->start.row >= 0);
xassert(iter->start.row < term->rows);
xassert(iter->start.col >= 0);
xassert(iter->start.col < term->cols);
struct coord abs_start = iter->start;
abs_start.row = grid_row_absolute_in_view(grid, abs_start.row);
struct coord abs_end = {
term->cols - 1,
grid_row_absolute_in_view(grid, term->rows - 1)};
found = find_next(term, SEARCH_FORWARD, abs_start, abs_end, &match);
}
if (found) {
LOG_DBG("match at %dx%d-%dx%d",
match.start.row, match.start.col,
match.end.row, match.end.col);
/* Convert absolute row numbers back to view relative */
match.start.row = match.start.row - grid->view + grid->num_rows;
match.start.row &= grid->num_rows - 1;
match.end.row = match.end.row - grid->view + grid->num_rows;
match.end.row &= grid->num_rows - 1;
xassert(match.start.row >= 0);
xassert(match.start.row < term->rows);
xassert(match.end.row >= 0);
xassert(match.end.row < term->rows);
xassert(match.end.row > match.start.row ||
(match.end.row == match.start.row &&
match.end.col >= match.start.col));
if (return_primary_match) {
iter->start.row = 0;
iter->start.col = 0;
} else {
/* Continue at next column, next time */
iter->start.row = match.start.row;
iter->start.col = match.start.col + 1;
if (iter->start.col >= term->cols) {
iter->start.col = 0;
iter->start.row++; /* Overflow is caught in next iteration */
}
xassert(iter->start.row >= 0);
xassert(iter->start.row <= term->rows);
xassert(iter->start.col >= 0);
xassert(iter->start.col < term->cols);
if (match.start.row == term->search.match.row &&
match.start.col == term->search.match.col)
{
/* Primary match is handled explicitly */
LOG_DBG("primary match: skipping");
return search_matches_next(iter);
}
}
return match;
}
xassert(iter->start.row >= 0);
xassert(iter->start.row <= term->rows);
xassert(iter->start.col >= 0);
xassert(iter->start.col < term->cols);
return match;
no_match:
iter->start.row = -1;
@ -663,15 +614,18 @@ search_match_to_end_of_word(struct terminal *term, bool spaces_only)
if (term->search.match_len == 0)
return;
xassert(term->selection.coords.end.row != -1);
xassert(term->selection.coords.end.row >= 0);
struct grid *grid = term->grid;
const bool move_cursor = term->search.cursor == term->search.len;
const struct coord old_end = term->selection.coords.end;
struct coord old_end = selection_get_end(term);
struct coord new_end = old_end;
struct row *row = NULL;
xassert(new_end.row >= 0);
xassert(new_end.row < grid->num_rows);
/* Advances a coordinate by one column, to the right. Returns
* false if we’ve reached the scrollback wrap-around */
#define advance_pos(coord) __extension__ \
@ -691,12 +645,16 @@ search_match_to_end_of_word(struct terminal *term, bool spaces_only)
if (!advance_pos(new_end))
return;
xassert(new_end.row >= 0);
xassert(new_end.row < grid->num_rows);
xassert(grid->rows[new_end.row] != NULL);
/* Find next word boundary */
new_end.row -= grid->view;
new_end.row -= grid->view + grid->num_rows;
new_end.row &= grid->num_rows - 1;
selection_find_word_boundary_right(term, &new_end, spaces_only);
new_end.row += grid->view;
new_end.row &= grid->num_rows - 1;
struct coord pos = old_end;
row = grid->rows[pos.row];