From 628c73d984fd7663607cc3fd9368f809855906fd Mon Sep 17 00:00:00 2001 From: Robin Haberkorn Date: Sun, 13 Apr 2025 00:40:37 +0300 Subject: fixed undoing bitfields on Windows * It turns out that `bool` (_Bool) in bitfields may cause padding to the next 32-bit word. This was only observed on MinGW. I am not entirely sure why, although the C standard does not guarantee much with regard to bitfield memory layout and there are 64-bit available due to passing anyway. Actually, they could also be layed out in a different order. * I am now consistently using guint instead of `bool` in bitfields to prevent any potential surprises. * The way that guint was aliased with bitfield structs for undoing teco_machine_main_t and teco_machine_qregspec_t flags was therefore insecure. It was not guaranteed that the __flags field really "captures" all of the bit field. Even with `guint v : 1` fields, this was not guaranteed. We would have required a static assertion for robustness. Alternatively, we could have declared a `gsize __flags` variable as well. This __should__ be safe since gsize should always be pointer sized and correspond to the platform's alignment. However, it's also not 100% guaranteed. Using classic ANSI C enums with bit operations to encode multiple fields and flags into a single integer also doesn't look very attractive. * Instead, we now define scalar types with their own teco_undo_push() shortcuts for the bitfield structs. This is in one way simpler and much more robust, but on the other hand complicates access to the flag variables. * It's a good question whether a `struct __attribute__((packed))` bitfield with guint fields would be a reliable replacement for flag enums, that are communicated with the "outside" (TECO) world. I am not going to risk it until GCC gives any guarantees, though. For the time being, bitfields are only used internally where the concrete memory layout (bit positions) is not crucial. * This fixes the test suite and therefore probably CI and nightly builds on Windows. * Test case: Rub out `@I//` or `@Xq` until before the `@`. The parser doesn't know that `@` is still set and allows all sorts of commands where `@` should be forbidden. * It's unknown how long this has been broken on Windows - quite possibly since v2.0. --- tests/testsuite.at | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'tests') diff --git a/tests/testsuite.at b/tests/testsuite.at index 3a33838..b0ca8c0 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -357,6 +357,12 @@ AT_CHECK([$SCITECO_CMDLINE "GaGb{-4D}"], 0, ignore, stderr) AT_FAIL_IF([$GREP "^Error:" stderr]) AT_CLEANUP +AT_SETUP([Restore flags after rub out]) +# Must throw an error if the @ flag is restored properly. +AT_CHECK([$SCITECO_CMDLINE '0@W{-D}C'], 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