From 33f71654136014bac094babaaa81d91245fdd24c Mon Sep 17 00:00:00 2001 From: Robin Haberkorn Date: Fri, 21 Mar 2025 13:26:01 +0300 Subject: fixed rubout of Q-Register specifications * This was a regression introduced by 257a0bf128e109442dce91c4aaa1d97fed17ad1a. * The undo token that frees newly allocated teco_machine_qregspec_t must actually reset the pointer as well since any subsequent token, pushed by teco_undo_qregspec_own(), will expect a valid pointer. * Could have been done via ctx->expectqreg = NULL; teco_undo_qregspec_own(ctx->expectqreg); but using a special clear function requires less memory and is easier to understand. * Added test case. This wouldn't always crash, but should definitely show up in Valgrind. --- src/qreg-commands.c | 4 ++-- src/qreg.c | 9 ++++++++- src/qreg.h | 2 +- src/undo.h | 1 + 4 files changed, 12 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/qreg-commands.c b/src/qreg-commands.c index ebf6caa..3da9c46 100644 --- a/src/qreg-commands.c +++ b/src/qreg-commands.c @@ -45,7 +45,7 @@ teco_state_expectqreg_initial(teco_machine_main_t *ctx, GError **error) ctx->expectqreg = teco_machine_qregspec_new(current->expectqreg.type, ctx->qreg_table_locals, ctx->parent.must_undo); if (ctx->parent.must_undo) - undo__teco_machine_qregspec_free(ctx->expectqreg); + undo__teco_machine_qregspec_clear(&ctx->expectqreg); return TRUE; } @@ -240,7 +240,7 @@ teco_state_queryqreg_initial(teco_machine_main_t *ctx, GError **error) ctx->expectqreg = teco_machine_qregspec_new(type, ctx->qreg_table_locals, ctx->parent.must_undo); if (ctx->parent.must_undo) - undo__teco_machine_qregspec_free(ctx->expectqreg); + undo__teco_machine_qregspec_clear(&ctx->expectqreg); return TRUE; } diff --git a/src/qreg.c b/src/qreg.c index 1b97689..39406a2 100644 --- a/src/qreg.c +++ b/src/qreg.c @@ -1715,5 +1715,12 @@ teco_machine_qregspec_free(teco_machine_qregspec_t *ctx) g_free(ctx); } -TECO_DEFINE_UNDO_CALL(teco_machine_qregspec_free, teco_machine_qregspec_t *); +static inline void +teco_machine_qregspec_clear(teco_machine_qregspec_t **ctx) +{ + teco_machine_qregspec_free(*ctx); + *ctx = NULL; +} + +TECO_DEFINE_UNDO_CALL(teco_machine_qregspec_clear, teco_machine_qregspec_t **); TECO_DEFINE_UNDO_OBJECT_OWN(qregspec, teco_machine_qregspec_t *, teco_machine_qregspec_free); diff --git a/src/qreg.h b/src/qreg.h index 4260202..7a9e13c 100644 --- a/src/qreg.h +++ b/src/qreg.h @@ -252,7 +252,7 @@ void teco_machine_qregspec_free(teco_machine_qregspec_t *ctx); G_DEFINE_AUTOPTR_CLEANUP_FUNC(teco_machine_qregspec_t, teco_machine_qregspec_free); /** @memberof teco_machine_qregspec_t */ -void undo__teco_machine_qregspec_free(teco_machine_qregspec_t *); +void undo__teco_machine_qregspec_clear(teco_machine_qregspec_t **); TECO_DECLARE_UNDO_OBJECT(qregspec, teco_machine_qregspec_t *); #define teco_undo_qregspec_own(VAR) \ diff --git a/src/undo.h b/src/undo.h index 6fbb1c1..459fdb0 100644 --- a/src/undo.h +++ b/src/undo.h @@ -143,6 +143,7 @@ gpointer teco_undo_push_size(teco_undo_action_t action_cb, gsize size) * At undo-time however, it is expected to be valid. * Therefore it is crucial to set the variable afterwards or at least to nullify it. * The DELETE function may consequently have to deal with NULL pointers. + * Also, any other undo action must always leave the variable in a valid state. * * @ingroup undo_objects */ -- cgit v1.2.3