From 6d2dc774947787dcef73a3251d1e9ec51bc526df Mon Sep 17 00:00:00 2001 From: Berke Güzel Date: Thu, 15 Oct 2026 12:42:11 +0300 Subject: Report selection allocation failure instead of truncating On a screen taller than 256 rows the selection's row list is allocated. When that failed, it fell back to the 256-entry stack array, so the selection and the snapshot's selection spans silently stopped at row 256 while every call still reported success. lines_init() now reports the failure. term_select_start() and term_select_extend() return false and leave the selection unchanged, term_snapshot_update() fails and keeps its previous content, and the text getters return NULL, as they do for their other allocations. Co-Authored-By: Claude Opus 5.5 --- include/term/term.h | 10 +++--- src/screen.h | 3 +- src/selection.c | 77 ++++++++++++++++++++++++++++++++----------- src/snapshot.c | 3 +- tests/snapshot_failure_test.c | 57 +++++++++++++++++++++++++++++--- 5 files changed, 119 insertions(+), 31 deletions(-) diff --git a/include/term/term.h b/include/term/term.h index c69e88e..8b98a25 100644 --- a/include/term/term.h +++ b/include/term/term.h @@ -336,11 +336,13 @@ enum term_select_unit { }; /* Start a selection at cell. For TERM_SELECT_CELL, right_half means the - * pointer is on the right half of the cell, so the selection starts after it. */ -TERM_API void term_select_start(struct term *t, struct term_vector2i cell, bool right_half, + * pointer is on the right half of the cell, so the selection starts after it. + * Returns false when memory runs out; the selection is then unchanged. */ +TERM_API bool term_select_start(struct term *t, struct term_vector2i cell, bool right_half, enum term_select_unit unit); -/* Extend the selection from its start to cell. */ -TERM_API void term_select_extend(struct term *t, struct term_vector2i cell, bool right_half); +/* Extend the selection from its start to cell. Returns false when memory runs + * out; the selection is then unchanged. */ +TERM_API bool term_select_extend(struct term *t, struct term_vector2i cell, bool right_half); TERM_API void term_select_clear(struct term *t); TERM_API bool term_has_selection(const struct term *t); /* Owned, NUL-terminated UTF-8 of the selection. NULL when nothing is diff --git a/src/screen.h b/src/screen.h index 0bd95c0..df464a7 100644 --- a/src/screen.h +++ b/src/screen.h @@ -18,7 +18,8 @@ void term_screen_insert_chars(struct term *t, int count); const struct cell *term_screen_view_row(const struct term *t, int y); uint64_t term_screen_view_row_key(const struct term *t, int y); /* The selected columns [c0, c1] of each view row; c1 < c0 when none. */ -void term_selection_view_spans(const struct term *t, int *c0, int *c1); +/* False when memory runs out; c0 and c1 are then not complete. */ +bool term_selection_view_spans(const struct term *t, int *c0, int *c1); int term_screen_history_rows(const struct term *t); diff --git a/src/selection.c b/src/selection.c index 40fb1f1..1d931c9 100644 --- a/src/selection.c +++ b/src/selection.c @@ -109,7 +109,9 @@ struct lines { struct line_pos stack[256]; }; -static void lines_init(struct lines *L, const struct term *t) +/* False when a screen taller than the stack array needs memory and none is + * left. L can still be passed to lines_free(). */ +static bool lines_init(struct lines *L, const struct term *t) { L->t = t; L->cols = t->cols; @@ -117,14 +119,16 @@ static void lines_init(struct lines *L, const struct term *t) L->hist_end = L->hist ? hist_end_id(&t->hist) : 0; L->first = first_retained_line(t); L->pos = t->rows <= 256 ? L->stack : malloc((size_t)t->rows * sizeof(*L->pos)); - if (!L->pos) + if (!L->pos) { L->pos = L->stack; - int rows = L->pos == L->stack && t->rows > 256 ? 256 : t->rows; + return false; + } struct line_pos p = screen_top_line(t); - for (int y = 0; y < rows; y++) { + for (int y = 0; y < t->rows; y++) { L->pos[y] = p; p = line_pos_next(p, screen_get_row(&t->scr, y)); } + return true; } static void lines_free(struct lines *L) @@ -135,7 +139,7 @@ static void lines_free(struct lines *L) static inline int lines_rows(const struct lines *L) { - return L->pos == L->stack && L->t->rows > 256 ? 256 : L->t->rows; + return L->t->rows; } static inline bool in_hist(const struct lines *L, uint64_t id) @@ -498,27 +502,48 @@ static void select_range(struct term *t, const struct lines *L, t->selection = true; } -void term_select_start(struct term *t, struct term_vector2i cell, bool right_half, +bool term_select_start(struct term *t, struct term_vector2i cell, bool right_half, enum term_select_unit unit) { - if (!t) return; + if (!t) return false; term_write_begin(t); struct lines L; - lines_init(&L, t); + if (!lines_init(&L, t)) { + lines_free(&L); + term_write_end(t); + return false; + } + struct term_sel_point first, last; + unit_span(&L, cell.x, cell.y, right_half, unit, &first, &last); + lines_free(&L); + enum term_select_unit old_unit = t->select_unit; + struct term_sel_point old_first = t->sel_anchor_first, old_last = t->sel_anchor_last; + bool old_selection = t->selection; t->select_unit = unit; - unit_span(&L, cell.x, cell.y, right_half, unit, &t->sel_anchor_first, &t->sel_anchor_last); + t->sel_anchor_first = first; + t->sel_anchor_last = last; t->selection = false; - lines_free(&L); - term_select_extend(t, cell, right_half); + bool ok = term_select_extend(t, cell, right_half); + if (!ok) { + t->select_unit = old_unit; + t->sel_anchor_first = old_first; + t->sel_anchor_last = old_last; + t->selection = old_selection; + } term_write_end(t); + return ok; } -void term_select_extend(struct term *t, struct term_vector2i cell, bool right_half) +bool term_select_extend(struct term *t, struct term_vector2i cell, bool right_half) { - if (!t) return; + if (!t) return false; term_write_begin(t); struct lines L; - lines_init(&L, t); + if (!lines_init(&L, t)) { + lines_free(&L); + term_write_end(t); + return false; + } struct term_sel_point first, last; unit_span(&L, cell.x, cell.y, right_half, t->select_unit, &first, &last); if (sel_point_cmp(first, t->sel_anchor_first) > 0) first = t->sel_anchor_first; @@ -529,6 +554,7 @@ void term_select_extend(struct term *t, struct term_vector2i cell, bool right_ha select_range(t, &L, first, sel_point_prev(&L, last)); lines_free(&L); term_write_end(t); + return true; } bool term_has_selection(const struct term *t) @@ -541,7 +567,7 @@ bool term_has_selection(const struct term *t) return has; } -void term_selection_view_spans(const struct term *t, int *c0, int *c1) +bool term_selection_view_spans(const struct term *t, int *c0, int *c1) { int rows = t->rows, cols = t->cols; for (int y = 0; y < rows; y++) { @@ -549,13 +575,16 @@ void term_selection_view_spans(const struct term *t, int *c0, int *c1) c1[y] = -1; } if (!t->selection) - return; + return true; struct lines L; - lines_init(&L, t); + if (!lines_init(&L, t)) { + lines_free(&L); + return false; + } struct term_sel_point start, end; if (!selection_bounds(&L, &start, &end)) { lines_free(&L); - return; + return true; } uint64_t prev_id = UINT64_MAX; int prev_seg = -1; @@ -596,6 +625,7 @@ void term_selection_view_spans(const struct term *t, int *c0, int *c1) c1[y] = (int)e; } lines_free(&L); + return true; } static bool text_reserve(char **text, size_t *cap, size_t need) @@ -619,7 +649,10 @@ static char *range_text_inner(const struct term *t, { if (!t || (!bounds && !t->selection)) return NULL; struct lines L; - lines_init(&L, t); + if (!lines_init(&L, t)) { + lines_free(&L); + return NULL; + } struct term_sel_point start, end; if (bounds) { start = bounds[0]; @@ -783,7 +816,11 @@ char *term_history_text(struct term *t, int max_rows) } int begin_row = end_row > max_rows ? end_row - max_rows : 0; struct lines L; - lines_init(&L, t); + if (!lines_init(&L, t)) { + lines_free(&L); + term_write_end(t); + return NULL; + } struct term_sel_point bounds[2] = {0}; int first_seg, last_seg; char *text = NULL; diff --git a/src/snapshot.c b/src/snapshot.c index 8726265..78c66b0 100644 --- a/src/snapshot.c +++ b/src/snapshot.c @@ -318,7 +318,8 @@ static bool term_snapshot_update_inner(struct term_snapshot *s, struct term *t) s->arena_cap = cap; } - term_selection_view_spans(t, c0, c1); + if (!term_selection_view_spans(t, c0, c1)) + goto done; p.all = all; p.scrolled = scrolled; apply(s, t, &p); diff --git a/tests/snapshot_failure_test.c b/tests/snapshot_failure_test.c index 6ce8f7b..7d4b139 100644 --- a/tests/snapshot_failure_test.c +++ b/tests/snapshot_failure_test.c @@ -327,9 +327,8 @@ static bool resize_at(int k) return injected; } -/* More than 256 rows moves the scratch keys and spans off the stack. The - * selection is left out: its own row list falls back to 256 rows when its - * allocation fails, which is outside the snapshot. */ +/* More than 256 rows moves the scratch keys and spans, and the selection's + * row list, off the stack. */ static bool tall_screen_at(int k) { struct term_options options = { .size = { 8, 300 }, .scrollback = 5 }; @@ -339,6 +338,9 @@ static bool tall_screen_at(int k) for (int y = 0; y < 299; y++) term_feed(t, "\r\n", 2, 0); term_feed(t, "e\xcc\x81yes", 5, 0); + assert(term_select_start(t, (struct term_vector2i){ .x = 0, .y = 299 }, false, + TERM_SELECT_CELL)); + assert(term_select_extend(t, (struct term_vector2i){ .x = 3, .y = 299 }, true)); bool injected = update_failing_at(s, t, k); @@ -346,6 +348,8 @@ static bool tall_screen_at(int k) const struct term_cell *row = term_snapshot_row(s, 299); uint32_t cps[4]; assert(cluster_cps(s, &row[0], cps, 4) == 2 && cps[0] == 'e' && cps[1] == 0x301); + struct term_span span = term_snapshot_row_selection(s, 299); + assert(span.begin == 0 && span.end == 4); expect_matches_fresh(t, s); term_snapshot_free(s); @@ -375,6 +379,47 @@ static void tall_selection_is_kept(void) term_free(t); } +/* On a screen taller than 256 rows, selecting needs memory. When none is + * left, the call fails and the selection stays as it was. */ +static void tall_selection_fails_cleanly(void) +{ + struct term_options options = { .size = { 8, 300 }, .scrollback = 5 }; + struct term *t = term_new(&options); + assert(t); + for (int y = 0; y < 299; y++) + term_feed(t, "\r\n", 2, 0); + term_feed(t, "yes", 3, 0); + assert(term_select_start(t, (struct term_vector2i){ .x = 0, .y = 299 }, false, + TERM_SELECT_CELL)); + assert(term_select_extend(t, (struct term_vector2i){ .x = 1, .y = 299 }, true)); + char *before = term_selection_text(t); + assert(before && !strcmp(before, "ye")); + + alloc_was_failed = false; + alloc_countdown = 0; + assert(!term_select_extend(t, (struct term_vector2i){ .x = 2, .y = 299 }, true)); + alloc_countdown = -1; + assert(alloc_was_failed); + + alloc_was_failed = false; + alloc_countdown = 0; + assert(!term_select_start(t, (struct term_vector2i){ .x = 2, .y = 299 }, false, + TERM_SELECT_WORD)); + alloc_countdown = -1; + assert(alloc_was_failed); + + char *after = term_selection_text(t); + assert(after && !strcmp(after, before)); + term_text_free(before); + term_text_free(after); + + assert(term_select_extend(t, (struct term_vector2i){ .x = 2, .y = 299 }, true)); + char *grown = term_selection_text(t); + assert(grown && !strcmp(grown, "yes")); + term_text_free(grown); + term_free(t); +} + /* Scroll with clusters in most lines. Each step fails every allocation in * turn, then retries, and must match a fresh capture. */ static void scrolling_fails_cleanly(void) @@ -417,10 +462,12 @@ int main(void) /* Four reserve() buffers and the arena, after the screen grows. */ failures = sweep(resize_at); assert(failures >= 5); - /* Four reserve() buffers, the scratch array, and the arena. */ + /* Four reserve() buffers, the scratch array, the selection's row list, + * and the arena. */ failures = sweep(tall_screen_at); - assert(failures >= 6); + assert(failures >= 7); tall_selection_is_kept(); + tall_selection_fails_cleanly(); scrolling_fails_cleanly(); return 0; } -- cgit v1.2.3