From 33cddff16ebd28d88db23f15cbc483e19daebcc8 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Tue, 12 May 2026 18:03:46 +1000 Subject: [PATCH 1/6] fix(windows): deep copy for state object This commit update the state object to have a deep copy for the action_struct member. The tests have been modified but have a few issues however this branch is out of date with the refacted master branch for unit tests. This commit gets the main change in a future commit will update the tests. --- core/src/action.cpp | 31 ++++ core/src/action.hpp | 4 + core/src/option.cpp | 15 ++ core/src/option.hpp | 5 +- core/src/state.cpp | 14 ++ core/src/state.hpp | 2 +- core/tests/unit/kmnkbd/state_api.tests.cpp | 175 ++++++++++++++++++++- 7 files changed, 241 insertions(+), 5 deletions(-) diff --git a/core/src/action.cpp b/core/src/action.cpp index 1ff7917575c..f9973110933 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -183,3 +183,34 @@ bool km::core::state::set_actions( return true; } +using namespace km::core; + +namespace { + +km_core_usv * duplicate_km_core_usv(const km_core_usv *src) { + if (!src) { + return nullptr; + } + size_t len = 0; + while (src[len]) { + ++len; + } + km_core_usv *result = new km_core_usv[len + 1]; + std::copy(src, src + len + 1, result); + return result; +} + +} // namespace + +km_core_actions km::core::clone_actions_object(km_core_actions const &src) { + km_core_actions result; + memset(&result, 0, sizeof(result)); + result.code_points_to_delete = src.code_points_to_delete; + result.do_alert = src.do_alert; + result.emit_keystroke = src.emit_keystroke; + result.new_caps_lock_state = src.new_caps_lock_state; + result.deleted_context = duplicate_km_core_usv(src.deleted_context); + result.output = duplicate_km_core_usv(src.output); + result.persist_options = clone_options(src.persist_options); + return result; +} diff --git a/core/src/action.hpp b/core/src/action.hpp index db1f6108519..220acbad2d7 100644 --- a/core/src/action.hpp +++ b/core/src/action.hpp @@ -39,5 +39,9 @@ namespace core unsigned int code_points_to_delete ); + km_core_actions clone_actions_object( + km_core_actions const &src + ); + } // namespace core } // namespace km diff --git a/core/src/option.cpp b/core/src/option.cpp index 6c66c93d3a1..53fbfaa50d0 100644 --- a/core/src/option.cpp +++ b/core/src/option.cpp @@ -82,3 +82,18 @@ json & km::core::operator << (json &j, abstract_processor const &) return j; } + +km_core_option_item * +km::core::clone_options(km_core_option_item const *src) { + if (!src) { + return nullptr; + } + size_t count = km_core_options_list_size(src); + km_core_option_item *result = new km_core_option_item[count + 1]; + for (size_t i = 0; i < count; ++i) { + km::core::option opt(static_cast(src[i].scope), src[i].key, src[i].value); + result[i] = opt.release(); + } + result[count] = KM_CORE_OPTIONS_END; + return result; +} diff --git a/core/src/option.hpp b/core/src/option.hpp index 60026798cfd..9c081d49a49 100644 --- a/core/src/option.hpp +++ b/core/src/option.hpp @@ -93,7 +93,10 @@ namespace core return key == nullptr; } - + /** + * Clones an array of km_core_option_item, performing deep copies of the strings. + */ + km_core_option_item * clone_options(km_core_option_item const *src); } // namespace core } // namespace km diff --git a/core/src/state.cpp b/core/src/state.cpp index 1ee68b9ed60..d0774b1e1f6 100644 --- a/core/src/state.cpp +++ b/core/src/state.cpp @@ -11,6 +11,7 @@ #include #include + using namespace km::core; void actions::push_persist(option const &opt) { @@ -85,6 +86,19 @@ void state::imx_callback(uint32_t imx_id) { _imx_callback(static_cast(this), imx_id, _imx_object); } +state::state(state const &other) + : _ctxt(other._ctxt) + , _app_ctxt(other._app_ctxt) + , _processor(other._processor) + , _actions(other._actions) + , _action_struct(clone_actions_object(other._action_struct)) + , _debug_items(other._debug_items) + , _imx_callback(other._imx_callback) + , _imx_object(other._imx_object) + , _backspace_handled_internally(other._backspace_handled_internally) +{ +} + state::~state() { km::core::actions_dispose(this->_action_struct); } diff --git a/core/src/state.hpp b/core/src/state.hpp index 0cf58fb614b..da2cf6bb75a 100644 --- a/core/src/state.hpp +++ b/core/src/state.hpp @@ -135,7 +135,7 @@ class state public: state(core::abstract_processor & kb, km_core_option_item const *env); - state(state const &) = default; + state(state const &other); state(state const &&) = delete; ~state(); diff --git a/core/tests/unit/kmnkbd/state_api.tests.cpp b/core/tests/unit/kmnkbd/state_api.tests.cpp index 70dcce31335..f165e7549c5 100644 --- a/core/tests/unit/kmnkbd/state_api.tests.cpp +++ b/core/tests/unit/kmnkbd/state_api.tests.cpp @@ -42,6 +42,134 @@ namespace return buf; } + inline + bool action_options_equal(km_core_option_item const * lhs, + km_core_option_item const * rhs) + { + if (lhs == rhs) return true; + if (!lhs || !rhs) return false; + + while (lhs->key && rhs->key) { + if (lhs->scope != rhs->scope) return false; + if (std::u16string(lhs->key) != std::u16string(rhs->key)) return false; + if (std::u16string(lhs->value) != std::u16string(rhs->value)) return false; + ++lhs; + ++rhs; + } + + return lhs->key == nullptr && rhs->key == nullptr; + } + + inline + bool expect_action_struct( + km_core_actions const & actions, + unsigned int expected_code_points_to_delete, + km_core_usv const * expected_output, + km_core_option_item const * expected_persist_options, + km_core_bool expected_do_alert, + km_core_bool expected_emit_keystroke, + km_core_caps_state expected_new_caps_lock_state, + km_core_usv const * expected_deleted_context + ) { + bool all_passed = true; + + std::cout << "\n=== Comparing action_struct fields ===" << std::endl; + + // code_points_to_delete + std::cout << "code_points_to_delete: " << actions.code_points_to_delete + << " (expected: " << expected_code_points_to_delete << ")"; + if (actions.code_points_to_delete != expected_code_points_to_delete) { + std::cout << " [FAIL]" << std::endl; + all_passed = false; + } else { + std::cout << " [PASS]" << std::endl; + } + + // output + std::cout << "output: " << (actions.output ? std::u32string(actions.output) : U"(null)") + << " expected: " << (expected_output ? std::u32string(expected_output) : U"(null)") << std::endl; + if (actions.output != expected_output) { + std::cout << " [FAIL]" << std::endl; + //all_passed = false; + } else { + std::cout << " [PASS]" << std::endl; + } + + // persist_options + std::cout << "persist_options comparison:" << std::endl; + if (!action_options_equal(actions.persist_options, expected_persist_options)) { + std::cout << " actual: "; + if (actions.persist_options) { + for (auto opt = actions.persist_options; opt->scope; ++opt) { + std::cout << "[scope=" << opt->scope << ", key=" << opt->key << ", value=" << opt->value << "] "; + } + } else { + std::cout << "(null)"; + } + std::cout << std::endl; + std::cout << " expected: "; + if (expected_persist_options) { + for (auto opt = expected_persist_options; opt->key; ++opt) { + std::cout << "[scope=" << opt->scope << ", key=" << opt->key << ", value=" << opt->value << "] "; + } + } else { + std::cout << "(null)"; + } + std::cout << " [FAIL]" << std::endl; + all_passed = false; + } else { + std::cout << " [PASS]" << std::endl; + } + + // do_alert + std::cout << "do_alert: " << (int)actions.do_alert + << " (expected: " << (int)expected_do_alert << ")"; + if (actions.do_alert != expected_do_alert) { + std::cout << " [FAIL]" << std::endl; + all_passed = false; + } else { + std::cout << " [PASS]" << std::endl; + } + + // emit_keystroke + std::cout << "emit_keystroke: " << (int)actions.emit_keystroke + << " (expected: " << (int)expected_emit_keystroke << ")"; + if (actions.emit_keystroke != expected_emit_keystroke) { + std::cout << " [FAIL]" << std::endl; + all_passed = false; + } else { + std::cout << " [PASS]" << std::endl; + } + + // new_caps_lock_state + std::cout << "new_caps_lock_state: " << (int)actions.new_caps_lock_state + << " (expected: " << (int)expected_new_caps_lock_state << ")"; + if (actions.new_caps_lock_state != expected_new_caps_lock_state) { + std::cout << " [FAIL]" << std::endl; + all_passed = false; + } else { + std::cout << " [PASS]" << std::endl; + } + + // deleted_context + std::cout << "deleted_context: " << (actions.deleted_context ? std::u32string(actions.deleted_context) : U"(null)") + << " (expected: " << (expected_deleted_context ? std::u32string(expected_deleted_context) : U"(null)") << ")"; + if (expected_deleted_context != actions.deleted_context) { + if (std::u32string(actions.deleted_context) != std::u32string(expected_deleted_context)) { + std::cout << " [FAIL]" << std::endl; + all_passed = false; + } else { + std::cout << " [PASS]" << std::endl; + } + } else { + std::cout << " [PASS]" << std::endl; + } + + std::cout << "=== End comparison ===" << std::endl << std::endl; + + return all_passed; + } + km_core_option_item test_env_opts[] = { {u"hello", u"world", 0}, @@ -92,6 +220,7 @@ constexpr km_core_option_item const expected_persist_opt = { KM_CORE_OPT_KEYBOARD }; + extern "C" { uint8_t test_imx_callback(km_core_state *state, uint32_t imx_id, void *callback_object){ @@ -187,10 +316,50 @@ int main(int argc, char * argv[]) if (doc1 != doc1_expected) return __LINE__; if (doc2 != doc2_expected) return __LINE__; + // Test the action_struct values for the active and cloned states. + const unsigned int expected_state_code_points_to_delete = 1; + const km_core_usv * expected_state_output = U" "; + const km_core_bool expected_state_do_alert = KM_CORE_FALSE; + const km_core_bool expected_state_emit_keystroke = KM_CORE_FALSE; + const km_core_caps_state expected_state_new_caps_lock_state = KM_CORE_CAPS_UNCHANGED; + km_core_option_item expected_options[] = {expected_persist_opt, KM_CORE_OPTIONS_END }; + const km_core_usv * expected_deleted_text = U"L"; + // Cloned expected values + const unsigned int clone_state_code_points_to_delete = 0; + const km_core_usv * clone_state_output = U""; + const km_core_bool clone_state_do_alert = KM_CORE_FALSE; + const km_core_bool clone_state_emit_keystroke = KM_CORE_FALSE; + const km_core_caps_state clone_state_new_caps_lock_state = KM_CORE_CAPS_UNCHANGED; + km_core_option_item clone_state_options[] = {KM_CORE_OPTIONS_END}; + const km_core_usv * clone_state_deleted_text = nullptr; + + const auto & state_actions = test_state->action_struct(); + const auto & clone_actions = test_clone->action_struct(); + + test_assert (expect_action_struct(state_actions, + expected_state_code_points_to_delete, + expected_state_output, + expected_options, + expected_state_do_alert, + expected_state_emit_keystroke, + expected_state_new_caps_lock_state, + expected_deleted_text // expected_deleted_context_null + )); + + test_assert (expect_action_struct(clone_actions, + clone_state_code_points_to_delete, + clone_state_output, + clone_state_options, + clone_state_do_alert, + clone_state_emit_keystroke, + clone_state_new_caps_lock_state, + clone_state_deleted_text // expected_deleted_context_null + )); + // Destroy them - km_core_state_dispose(test_state); - km_core_state_dispose(test_clone); - km_core_keyboard_dispose(test_kb); + km_core_state_dispose(test_state); + km_core_state_dispose(test_clone); + km_core_keyboard_dispose(test_kb); return 0; From 3c0e7a72c14c6cdc31015496d9ec2c694071b647 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Thu, 14 May 2026 15:58:12 +1000 Subject: [PATCH 2/6] fix(core): update unit tests --- core/tests/unit/kmnkbd/state_api.tests.cpp | 27 ++++++++++++++-------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/core/tests/unit/kmnkbd/state_api.tests.cpp b/core/tests/unit/kmnkbd/state_api.tests.cpp index f165e7549c5..f39c7e36ee8 100644 --- a/core/tests/unit/kmnkbd/state_api.tests.cpp +++ b/core/tests/unit/kmnkbd/state_api.tests.cpp @@ -88,9 +88,11 @@ namespace // output std::cout << "output: " << (actions.output ? std::u32string(actions.output) : U"(null)") << " expected: " << (expected_output ? std::u32string(expected_output) : U"(null)") << std::endl; - if (actions.output != expected_output) { - std::cout << " [FAIL]" << std::endl; - //all_passed = false; + if (expected_output != actions.output) { + if (std::u32string(actions.output) != std::u32string(expected_output)) { + std::cout << " [FAIL]" << std::endl; + all_passed = false; + } } else { std::cout << " [PASS]" << std::endl; } @@ -297,6 +299,11 @@ int main(int argc, char * argv[]) try_status(km_core_process_event(test_state, KM_CORE_VKEY_L, KM_CORE_MODIFIER_SHIFT, 1, KM_CORE_EVENT_FLAG_DEFAULT)); test_assert(action_items(test_state, {{KM_CORE_IT_CHAR, {0,}, {km_core_usv('L')}}, {KM_CORE_IT_END}})); + + // Without the calling `km_core_state_context_set_if_needed` action struct has a delete for 'L'? + km_core_cu const *state_context = get_context_as_string(km_core_state_context(test_state)); + km_core_state_context_set_if_needed(test_state, state_context); + try_status(km_core_process_event(test_state, KM_CORE_VKEY_F2, 0, 1, KM_CORE_EVENT_FLAG_DEFAULT)); km_core_action_item action = {KM_CORE_IT_PERSIST_OPT, {0,}, }; @@ -316,14 +323,15 @@ int main(int argc, char * argv[]) if (doc1 != doc1_expected) return __LINE__; if (doc2 != doc2_expected) return __LINE__; - // Test the action_struct values for the active and cloned states. - const unsigned int expected_state_code_points_to_delete = 1; - const km_core_usv * expected_state_output = U" "; + // Test the action_struct values for the active and cloned states are + // independent and match their respective expected values. + const unsigned int expected_state_code_points_to_delete = 0; + const km_core_usv * expected_state_output = U""; const km_core_bool expected_state_do_alert = KM_CORE_FALSE; const km_core_bool expected_state_emit_keystroke = KM_CORE_FALSE; const km_core_caps_state expected_state_new_caps_lock_state = KM_CORE_CAPS_UNCHANGED; km_core_option_item expected_options[] = {expected_persist_opt, KM_CORE_OPTIONS_END }; - const km_core_usv * expected_deleted_text = U"L"; + const km_core_usv * expected_deleted_text = U""; // Cloned expected values const unsigned int clone_state_code_points_to_delete = 0; const km_core_usv * clone_state_output = U""; @@ -343,7 +351,7 @@ int main(int argc, char * argv[]) expected_state_do_alert, expected_state_emit_keystroke, expected_state_new_caps_lock_state, - expected_deleted_text // expected_deleted_context_null + expected_deleted_text )); test_assert (expect_action_struct(clone_actions, @@ -353,7 +361,7 @@ int main(int argc, char * argv[]) clone_state_do_alert, clone_state_emit_keystroke, clone_state_new_caps_lock_state, - clone_state_deleted_text // expected_deleted_context_null + clone_state_deleted_text )); // Destroy them @@ -361,6 +369,5 @@ int main(int argc, char * argv[]) km_core_state_dispose(test_clone); km_core_keyboard_dispose(test_kb); - return 0; } From c0a29a32f7de5c97d1fcc62390beb10d713bffa3 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Thu, 14 May 2026 16:28:59 +1000 Subject: [PATCH 3/6] fix(core): remove white space and comments --- core/src/option.hpp | 3 --- core/src/state.cpp | 1 - core/tests/unit/kmnkbd/state_api.tests.cpp | 1 - 3 files changed, 5 deletions(-) diff --git a/core/src/option.hpp b/core/src/option.hpp index 9c081d49a49..56e231c3029 100644 --- a/core/src/option.hpp +++ b/core/src/option.hpp @@ -93,9 +93,6 @@ namespace core return key == nullptr; } - /** - * Clones an array of km_core_option_item, performing deep copies of the strings. - */ km_core_option_item * clone_options(km_core_option_item const *src); } // namespace core diff --git a/core/src/state.cpp b/core/src/state.cpp index d0774b1e1f6..8eb99d1b0a7 100644 --- a/core/src/state.cpp +++ b/core/src/state.cpp @@ -11,7 +11,6 @@ #include #include - using namespace km::core; void actions::push_persist(option const &opt) { diff --git a/core/tests/unit/kmnkbd/state_api.tests.cpp b/core/tests/unit/kmnkbd/state_api.tests.cpp index f39c7e36ee8..4e9fbcd9114 100644 --- a/core/tests/unit/kmnkbd/state_api.tests.cpp +++ b/core/tests/unit/kmnkbd/state_api.tests.cpp @@ -222,7 +222,6 @@ constexpr km_core_option_item const expected_persist_opt = { KM_CORE_OPT_KEYBOARD }; - extern "C" { uint8_t test_imx_callback(km_core_state *state, uint32_t imx_id, void *callback_object){ From 29b12ba42e7d59ec30a9a41a119823b992334774 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Mon, 18 May 2026 14:10:22 +1000 Subject: [PATCH 4/6] fix(core): fix memory leak in state_api test --- core/tests/unit/kmnkbd/state_api.tests.cpp | 36 ++++++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/core/tests/unit/kmnkbd/state_api.tests.cpp b/core/tests/unit/kmnkbd/state_api.tests.cpp index 4e9fbcd9114..9b46e2fea8d 100644 --- a/core/tests/unit/kmnkbd/state_api.tests.cpp +++ b/core/tests/unit/kmnkbd/state_api.tests.cpp @@ -88,11 +88,21 @@ namespace // output std::cout << "output: " << (actions.output ? std::u32string(actions.output) : U"(null)") << " expected: " << (expected_output ? std::u32string(expected_output) : U"(null)") << std::endl; - if (expected_output != actions.output) { + bool output_equal = false; + if (expected_output == actions.output) { // nullptr or same pointer + output_equal = true; + } else if (!expected_output || !actions.output) { + output_equal = false; + } else if (expected_output && actions.output) { if (std::u32string(actions.output) != std::u32string(expected_output)) { - std::cout << " [FAIL]" << std::endl; - all_passed = false; + output_equal = false; + } else { + output_equal = true; } + } + if (!output_equal) { + std::cout << " [FAIL]" << std::endl; + all_passed = false; } else { std::cout << " [PASS]" << std::endl; } @@ -166,6 +176,25 @@ namespace } else { std::cout << " [PASS]" << std::endl; } + bool deleted_equal = false; + if (expected_deleted_context == actions.deleted_context) { // nullptr or same pointer + deleted_equal = true; + } else if (!expected_deleted_context || !actions.deleted_context) { + deleted_equal = false; + } else if (expected_deleted_context && actions.deleted_context) { + if (std::u32string(actions.deleted_context) != std::u32string(expected_deleted_context)) { + deleted_equal = false; + } else { + deleted_equal = true; + } + } + if (!deleted_equal) { + std::cout << " [FAIL]" << std::endl; + all_passed = false; + } else { + std::cout << " [PASS]" << std::endl; + } + std::cout << "=== End comparison ===" << std::endl << std::endl; @@ -302,6 +331,7 @@ int main(int argc, char * argv[]) // Without the calling `km_core_state_context_set_if_needed` action struct has a delete for 'L'? km_core_cu const *state_context = get_context_as_string(km_core_state_context(test_state)); km_core_state_context_set_if_needed(test_state, state_context); + delete [] state_context; try_status(km_core_process_event(test_state, KM_CORE_VKEY_F2, 0, 1, KM_CORE_EVENT_FLAG_DEFAULT)); From 3b6e0a8da121d6d1c04651ce3b40e3a0aa0d07cb Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Mon, 18 May 2026 21:18:25 +1000 Subject: [PATCH 5/6] fix(windows): add comment to capture issue in test code --- core/tests/unit/kmnkbd/state_api.tests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/core/tests/unit/kmnkbd/state_api.tests.cpp b/core/tests/unit/kmnkbd/state_api.tests.cpp index 9b46e2fea8d..f17e4480802 100644 --- a/core/tests/unit/kmnkbd/state_api.tests.cpp +++ b/core/tests/unit/kmnkbd/state_api.tests.cpp @@ -329,6 +329,7 @@ int main(int argc, char * argv[]) test_assert(action_items(test_state, {{KM_CORE_IT_CHAR, {0,}, {km_core_usv('L')}}, {KM_CORE_IT_END}})); // Without the calling `km_core_state_context_set_if_needed` action struct has a delete for 'L'? + // Issue raised to investigate further: https://github.com/keymanapp/keyman/issues/15962 km_core_cu const *state_context = get_context_as_string(km_core_state_context(test_state)); km_core_state_context_set_if_needed(test_state, state_context); delete [] state_context; From 4e4dfe727562c80ba132e6d06f95e0de1d7ee841 Mon Sep 17 00:00:00 2001 From: rc-swag <58423624+rc-swag@users.noreply.github.com> Date: Tue, 19 May 2026 14:01:06 +1000 Subject: [PATCH 6/6] fix(windows): add cstring include to action.cpp --- core/src/action.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/action.cpp b/core/src/action.cpp index f9973110933..ca6b0b30d8d 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include "action.hpp"