From 13281f327bcbe4a4799bea2122b77829f0004774 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 29 Aug 2022 20:46:19 +0200 Subject: [PATCH 1/4] =?UTF-8?q?grid:=20when=20setting=20the=20new=20viewpo?= =?UTF-8?q?rt,=20ensure=20it=E2=80=99s=20correctly=20bounded?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Do this by using scrollback relative coordinates, and ensure the new viewport is not larger than (grid_rows - screen_rows), as that would mean the viewport crosses the scrollback wrap-around. --- grid.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/grid.c b/grid.c index f40803e1..f229effa 100644 --- a/grid.c +++ b/grid.c @@ -1016,23 +1016,6 @@ grid_resize_and_reflow( new_grid[idx] = grid_row_alloc(new_cols, true); } - grid->view = view_follows ? grid->offset : viewport.row; - - /* If enlarging the window, the old viewport may be too far down, - * with unallocated rows. Make sure this cannot happen */ - while (true) { - int idx = (grid->view + new_screen_rows - 1) & (new_rows - 1); - if (new_grid[idx] != NULL) - break; - grid->view--; - if (grid->view < 0) - grid->view += new_rows; - } - for (size_t r = 0; r < new_screen_rows; r++) { - int UNUSED idx = (grid->view + r) & (new_rows - 1); - xassert(new_grid[idx] != NULL); - } - /* Free old grid (rows already free:d) */ free(grid->rows); @@ -1040,6 +1023,17 @@ grid_resize_and_reflow( grid->num_rows = new_rows; grid->num_cols = new_cols; + /* + * Set new viewport, making sure it’s not too far down. + * + * This is done by using scrollback-start relative cooardinates, + * and bounding the new viewport to (grid_rows - screen_rows). + */ + int sb_view = grid_row_abs_to_sb( + grid, new_screen_rows, view_follows ? grid->offset : viewport.row); + grid->view = grid_row_sb_to_abs( + grid, new_screen_rows, min(sb_view, new_rows - new_screen_rows)); + /* Convert absolute coordinates to screen relative */ cursor.row -= grid->offset; while (cursor.row < 0) From 5c86358cd1f830c01333326423f862879f9cd6a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 29 Aug 2022 20:47:33 +0200 Subject: [PATCH 2/4] =?UTF-8?q?grid:=20reflow:=20don=E2=80=99t=20line-wrap?= =?UTF-8?q?=20the=20last=20row?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this patch, we would line-wrap the last row, just like any other row, and then afterwards try to reverse this, by adjusting the offset and free:ing and NULL:ing the "last row". The problem with this is if the scrollback is full. In this case, the row we’re freeing is the first row in the scrollback history. This means we’ll crash as soon as the viewport is moved to the top of the scrollback. The fix is fairly, simple. Skip the post-processing logic, and instead detect when we’re line-wrapping the last row, and skip the call to line_wrap(). This way, the last row in the new grid corresponds to the last row in the old grid. --- grid.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/grid.c b/grid.c index f229effa..bbac3395 100644 --- a/grid.c +++ b/grid.c @@ -935,13 +935,14 @@ grid_resize_and_reflow( start += cols; } - if (old_row->linebreak) { /* Erase the remaining cells */ memset(&new_row->cells[new_col_idx], 0, (new_cols - new_col_idx) * sizeof(new_row->cells[0])); new_row->linebreak = true; - line_wrap(); + + if (r + 1 < old_rows) + line_wrap(); } grid_row_free(old_grid[old_row_idx]); @@ -985,25 +986,6 @@ grid_resize_and_reflow( /* Set offset such that the last reflowed row is at the bottom */ grid->offset = new_row_idx - new_screen_rows + 1; - if (new_col_idx == 0) { - int next_to_last_new_row_idx = new_row_idx - 1; - next_to_last_new_row_idx += new_rows; - next_to_last_new_row_idx &= new_rows - 1; - - const struct row *next_to_last_row = new_grid[next_to_last_new_row_idx]; - if (next_to_last_row != NULL && next_to_last_row->linebreak) { - /* - * The next to last row is actually the *last* row. But we - * ended the reflow with a line-break, causing an empty - * row to be inserted at the bottom. Undo this. - */ - /* TODO: can we detect this in the reflow loop above instead? */ - grid->offset--; - grid_row_free(new_grid[new_row_idx]); - new_grid[new_row_idx] = NULL; - } - } - while (grid->offset < 0) grid->offset += new_rows; while (new_grid[grid->offset] == NULL) From 454c82f0f59fea5307fbb570954db26f05ddbd09 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 29 Aug 2022 21:03:21 +0200 Subject: [PATCH 3/4] =?UTF-8?q?grid:=20reflow:=20assert=20there=20aren?= =?UTF-8?q?=E2=80=99t=20any=20open=20URIs=20on=20the=20last=20row?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- grid.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/grid.c b/grid.c index bbac3395..7bfef5cb 100644 --- a/grid.c +++ b/grid.c @@ -943,6 +943,21 @@ grid_resize_and_reflow( if (r + 1 < old_rows) line_wrap(); + else if (new_row->extra != NULL && + new_row->extra->uri_ranges.count > 0) + { + /* + * line_wrap() "closes" still-open URIs. Since this is + * the *last* row, and since we’re line-breaking due + * to a hard line-break (rather than running out of + * cells in the "new_row"), there shouldn’t be an open + * URI (it would have been closed when we reached the + * end of the URI while reflowing the last "old" + * row). + */ + uint32_t last_idx = new_row->extra->uri_ranges.count - 1; + xassert(new_row->extra->uri_ranges.v[last_idx].end >= 0); + } } grid_row_free(old_grid[old_row_idx]); From 524474728a5c9a1228500ab39b4974469b156c4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Ekl=C3=B6f?= Date: Mon, 29 Aug 2022 21:04:56 +0200 Subject: [PATCH 4/4] changelog: crash when resizing window, or scrolling --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2eddc3bf..24403770 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,10 @@ subsequent motion and leave events. * Regression: “random” selected empty cells being highlighted as selected when they should not. +* Crash when either resizing the terminal window, or scrolling in the + scrollback history ([#1074][1074]) + +[1074]: https://codeberg.org/dnkl/foot/pulls/1074 ### Security