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 + tests/testsuite.at | 6 ++++++ 5 files changed, 18 insertions(+), 4 deletions(-) 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 */ diff --git a/tests/testsuite.at b/tests/testsuite.at index 86f8331..1c42fe9 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -310,6 +310,12 @@ AT_CHECK([$SCITECO_CMDLINE "@I/F/ J @I/X/ @FK/F/{-6D} Z-2\"N(0/0)'"], 0, ignore, AT_FAIL_IF([$GREP "^Error:" stderr]) AT_CLEANUP +AT_SETUP([Rub out Q-Register specifications]) +# This was causing memory corruptions, that would at least show up under Valgrind. +AT_CHECK([$SCITECO_CMDLINE "GaGb{-4D}"], 0, ignore, stderr) +AT_FAIL_IF([$GREP "^Error:" stderr]) +AT_CLEANUP + AT_SETUP([Searches from macro calls]) AT_CHECK([$SCITECO_CMDLINE "@^Um{:@S/XXX/} :Mm\"S(0/0)' Mm\"S(0/0)'"], 0, ignore, stderr) AT_FAIL_IF([$GREP "^Error:" stderr]) -- cgit v1.2.3