From 12310418b0a577fc0b60f0aae60503462073eb75 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 24 Jun 2023 19:49:02 +0200 Subject: Allow map/unmap during mapping execution Commits e49c0fb04 (unmap: fail if the mapping is currently executing, 2023-05-14) 42be0057a (map: fail if key is currently executing, 2023-06-24) fixed potential use-after-free issues. By doing so, it broke configurations that in practice have not triggered any crashes [1] [2]. For example with, set -remove global autocomplete insert hook global InsertCompletionShow .* %{ map window insert } hook global InsertCompletionHide .* %{ unmap window insert } The execution of the mapping triggers InsertCompletionHide fails at unmapping. This seems legit and I don't see an obvious alternative way to write it (InsertIdle would not be correct though it would work in practice). Fix the regression by allowing map and unmap again while keeping the mappings alive until they have finished executing. Applying map/unmap immediately seems like the most obvious semantics. Alternatively, we could apply them in between key presses. [1]: [2]: --- src/keymap_manager.cc | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'src/keymap_manager.cc') diff --git a/src/keymap_manager.cc b/src/keymap_manager.cc index 26579bf7..582a271d 100644 --- a/src/keymap_manager.cc +++ b/src/keymap_manager.cc @@ -13,20 +13,12 @@ namespace Kakoune void KeymapManager::map_key(Key key, KeymapMode mode, KeyList mapping, String docstring) { - if (auto it = m_mapping.find(KeyAndMode{key, mode}); it != m_mapping.end()) - if (it->value.is_executing) - throw runtime_error("cannot map key that is currently executing"); m_mapping[KeyAndMode{key, mode}] = {std::move(mapping), std::move(docstring)}; } void KeymapManager::unmap_key(Key key, KeymapMode mode) { - auto it = m_mapping.find(KeyAndMode{key, mode}); - if (it == m_mapping.end()) - return; - if (it->value.is_executing) - throw runtime_error("cannot unmap key that is currently executing"); - m_mapping.remove(it); + m_mapping.remove(KeyAndMode{key, mode}); } void KeymapManager::unmap_keys(KeymapMode mode) @@ -48,8 +40,8 @@ bool KeymapManager::is_mapped(Key key, KeymapMode mode) const (m_parent and m_parent->is_mapped(key, mode)); } -KeymapManager::KeymapInfo& -KeymapManager::get_mapping(Key key, KeymapMode mode) +const KeymapManager::KeymapInfo& +KeymapManager::get_mapping(Key key, KeymapMode mode) const { auto it = m_mapping.find(KeyAndMode{key, mode}); if (it != m_mapping.end()) -- cgit v1.2.3