From e49c0fb04095a2a1c546fd033ce2a1a6df3eb8d0 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 14 May 2023 12:59:18 +0200 Subject: [PATCH] unmap: fail if the mapping is currently executing When unmapping a key sequence that is currently executing, we continue executing freed memory which can have weird effects. Let's instead throw an error if that happens. In future we can support unmap in this scenario. Closes #4896 --- src/commands.cc | 21 +++++++++++++------ src/input_handler.cc | 5 ++++- src/keymap_manager.cc | 4 ++-- src/keymap_manager.hh | 4 +++- src/normal.cc | 1 + .../4896-unmap-executing-mapping/cmd | 1 + .../4896-unmap-executing-mapping/in | 1 + .../4896-unmap-executing-mapping/out | 1 + .../4896-unmap-executing-mapping/rc | 1 + 9 files changed, 29 insertions(+), 10 deletions(-) create mode 100644 test/regression/4896-unmap-executing-mapping/cmd create mode 100644 test/regression/4896-unmap-executing-mapping/in create mode 100644 test/regression/4896-unmap-executing-mapping/out create mode 100644 test/regression/4896-unmap-executing-mapping/rc diff --git a/src/commands.cc b/src/commands.cc index 4d659acd..84fe8361 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -1933,10 +1933,15 @@ const CommandDesc unmap_key_cmd = { if (key.size() != 1) throw runtime_error("only a single key can be unmapped"); - if (keymaps.is_mapped(key[0], keymap_mode) and - (parser.positional_count() < 4 or - (keymaps.get_mapping(key[0], keymap_mode).keys == - parse_keys(parser[3])))) + if (not keymaps.is_mapped(key[0], keymap_mode)) + return; + auto& mapping = keymaps.get_mapping(key[0], keymap_mode); + + if (mapping.is_executing) + throw runtime_error("cannot unmap key that is currently executing"); + + if (parser.positional_count() < 4 or + (mapping.keys == parse_keys(parser[3]))) keymaps.unmap_key(key[0], keymap_mode); } }; @@ -2653,8 +2658,12 @@ void enter_user_mode(Context& context, String mode_name, KeymapMode mode, bool l InputHandler::ScopedForceNormal force_normal{context.input_handler(), {}}; ScopedEdition edition(context); - for (auto& key : mapping.keys) - context.input_handler().handle_key(key); + + { + ScopedSetBool executing_mapping{mapping.is_executing}; + for (auto& key : mapping.keys) + context.input_handler().handle_key(key); + } if (lock) enter_user_mode(context, std::move(mode_name), mode, true); diff --git a/src/input_handler.cc b/src/input_handler.cc index 129a7e03..fb1d9ff6 100644 --- a/src/input_handler.cc +++ b/src/input_handler.cc @@ -1773,7 +1773,10 @@ void InputHandler::handle_key(Key key) if (keymaps.is_mapped(key, keymap_mode) and not m_context.keymaps_disabled()) { ScopedSetBool disable_history{context().history_disabled()}; - for (auto& k : keymaps.get_mapping(key, keymap_mode).keys) + + auto& mapping = keymaps.get_mapping(key, keymap_mode); + ScopedSetBool executing_mapping{mapping.is_executing}; + for (auto& k : mapping.keys) process_key(k); } else diff --git a/src/keymap_manager.cc b/src/keymap_manager.cc index 582a271d..baec1373 100644 --- a/src/keymap_manager.cc +++ b/src/keymap_manager.cc @@ -40,8 +40,8 @@ bool KeymapManager::is_mapped(Key key, KeymapMode mode) const (m_parent and m_parent->is_mapped(key, mode)); } -const KeymapManager::KeymapInfo& -KeymapManager::get_mapping(Key key, KeymapMode mode) const +KeymapManager::KeymapInfo& +KeymapManager::get_mapping(Key key, KeymapMode mode) { auto it = m_mapping.find(KeyAndMode{key, mode}); if (it != m_mapping.end()) diff --git a/src/keymap_manager.hh b/src/keymap_manager.hh index 7474ed95..ddb374e4 100644 --- a/src/keymap_manager.hh +++ b/src/keymap_manager.hh @@ -6,6 +6,7 @@ #include "hash.hh" #include "string.hh" #include "hash_map.hh" +#include "utils.hh" #include "vector.hh" namespace Kakoune @@ -42,8 +43,9 @@ public: { KeyList keys; String docstring; + NestedBool is_executing{}; }; - const KeymapInfo& get_mapping(Key key, KeymapMode mode) const; + KeymapInfo& get_mapping(Key key, KeymapMode mode); using UserModeList = Vector; UserModeList& user_modes() { diff --git a/src/normal.cc b/src/normal.cc index f1123ca7..87bf272a 100644 --- a/src/normal.cc +++ b/src/normal.cc @@ -2033,6 +2033,7 @@ void exec_user_mappings(Context& context, NormalParams params) ScopedEdition edition(context); ScopedSelectionEdition selection_edition{context}; + ScopedSetBool executing_mapping{mapping.is_executing}; for (auto& key : mapping.keys) context.input_handler().handle_key(key); }, "user mapping", diff --git a/test/regression/4896-unmap-executing-mapping/cmd b/test/regression/4896-unmap-executing-mapping/cmd new file mode 100644 index 00000000..2e0bedd2 --- /dev/null +++ b/test/regression/4896-unmap-executing-mapping/cmd @@ -0,0 +1 @@ +u diff --git a/test/regression/4896-unmap-executing-mapping/in b/test/regression/4896-unmap-executing-mapping/in new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/test/regression/4896-unmap-executing-mapping/in @@ -0,0 +1 @@ + diff --git a/test/regression/4896-unmap-executing-mapping/out b/test/regression/4896-unmap-executing-mapping/out new file mode 100644 index 00000000..190a1803 --- /dev/null +++ b/test/regression/4896-unmap-executing-mapping/out @@ -0,0 +1 @@ +123 diff --git a/test/regression/4896-unmap-executing-mapping/rc b/test/regression/4896-unmap-executing-mapping/rc new file mode 100644 index 00000000..6aaf7b52 --- /dev/null +++ b/test/regression/4896-unmap-executing-mapping/rc @@ -0,0 +1 @@ +map global user u %{i123:unmap global user ui456}