Skip to content

Commit 32bb1c8

Browse files
committed
fix(core): fix iterator in backspace handling
This change replaces the reverse iterator loop that holds a stale iterator across `pop_back()` calls with a pattern that directly accesses `context.back()` on each iteration. This avoids undefined behavior from iterator invalidation when mutating the list or vector. While so far the previous code didn't show problems, it might still access released memory depending on the implementation. The documentation for `pop_back()` says "References and iterators to the erased element are invalidated", so the previous implementation was clearly wrong. Test-bot: skip
1 parent 6540886 commit 32bb1c8

File tree

2 files changed

+14
-15
lines changed

2 files changed

+14
-15
lines changed

core/src/ldml/ldml_processor.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,15 +233,15 @@ void ldml_event_state::emit_backspace() {
233233
// Find out what the last actual character was and remove it.
234234
// attempt to get the last char
235235
// TODO-LDML: emoji backspace
236-
auto end = state->context().rbegin();
237-
while (end != state->context().rend()) {
238-
if (end->type == KM_CORE_CT_CHAR) {
236+
while (!state->context().empty()) {
237+
auto end = state->context().back();
238+
if (end.type == KM_CORE_CT_CHAR) {
239239
actions.code_points_to_delete++;
240240
state->context().pop_back();
241241
return;
242242
}
243243
// else loop again
244-
assert(end->type != KM_CORE_CT_END); // inappropriate here.
244+
assert(end.type != KM_CORE_CT_END); // inappropriate here.
245245
state->context().pop_back();
246246
}
247247
/*

core/tests/unit/ldml/ldml.cpp

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -157,21 +157,20 @@ apply_action(
157157
matched_text = true; // no text to match as context is empty.
158158
}
159159
// now, we need to simulate what ldml_processor::emit_backspace() is going to do.
160-
auto end = context.rbegin();
161-
while (end != context.rend()) {
162-
if (end->type == KM_CORE_CT_CHAR) {
163-
test_assert(!matched_text);
164-
test_assert_equal(end->character, ch); // expect popped char to be same as what's in context
165-
matched_text = true;
166-
context.pop_back();
167-
break; // exit on first real char
168-
}
169-
test_assert(end->type != KM_CORE_CT_END); // inappropriate here.
160+
while (!context.empty()) {
161+
if (context.back().type == KM_CORE_CT_CHAR) {
162+
test_assert(!matched_text);
163+
test_assert_equal(context.back().character, ch); // expect popped char to be same as what's in context
164+
matched_text = true;
170165
context.pop_back();
166+
break; // exit on first real char
171167
}
172-
test_assert(matched_text);
168+
test_assert(context.back().type != KM_CORE_CT_END); // inappropriate here.
169+
context.pop_back();
173170
}
171+
test_assert(matched_text);
174172
break;
173+
}
175174
case KM_CORE_IT_PERSIST_OPT:
176175
std::cout << " + TODO-LDML: persist_opt()" << std::endl;
177176
break;

0 commit comments

Comments
 (0)