From: Francis De Brabandere <francisdb@gmail.com> put_Key implemented a rename as an Add of the new key followed by a Remove of the old one, which moved the renamed key to the end of the enumeration order and, when the source key was absent, silently created a new entry. Rename the existing pair in place instead, preserving its position, and return CTL_E_ELEMENT_NOT_FOUND when the source key does not exist. --- dlls/scrrun/dictionary.c | 39 +++++++++++++++++++++++----------- dlls/scrrun/tests/dictionary.c | 10 ++++----- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/dlls/scrrun/dictionary.c b/dlls/scrrun/dictionary.c index af771c9245a..6ae136c518f 100644 --- a/dlls/scrrun/dictionary.c +++ b/dlls/scrrun/dictionary.c @@ -681,25 +681,40 @@ static HRESULT WINAPI dictionary_put_Key(IDictionary *iface, VARIANT *key, VARIA { struct dictionary *dictionary = impl_from_IDictionary(iface); struct keyitem_pair *pair; - VARIANT empty; + VARIANT new_key, hash; + struct list *head; HRESULT hr; TRACE("%p, %s, %s.\n", iface, debugstr_variant(key), debugstr_variant(newkey)); - if ((pair = get_keyitem_pair(dictionary, key))) - { - /* found existing pair for a key, add new pair with new key - and old item and remove old pair after that */ + if (!(pair = get_keyitem_pair(dictionary, key))) + return CTL_E_ELEMENT_NOT_FOUND; - hr = IDictionary_Add(iface, newkey, &pair->item); - if (FAILED(hr)) - return hr; + /* The key is renamed in place, preserving its value and enumeration + position; only collisions with a different pair are rejected. */ + if (get_keyitem_pair(dictionary, newkey) && get_keyitem_pair(dictionary, newkey) != pair) + return CTL_E_KEY_ALREADY_EXISTS; - return IDictionary_Remove(iface, key); - } + hr = IDictionary_get_HashVal(iface, newkey, &hash); + if (FAILED(hr)) + return hr; - VariantInit(&empty); - return IDictionary_Add(iface, newkey, &empty); + VariantInit(&new_key); + hr = VariantCopyInd(&new_key, newkey); + if (FAILED(hr)) + return hr; + + VariantClear(&pair->key); + pair->key = new_key; + pair->hash = V_I4(&hash); + + list_remove(&pair->bucket); + head = get_bucket_head(dictionary, pair->hash); + if (!head->next) + list_init(head); + list_add_tail(head, &pair->bucket); + + return S_OK; } static HRESULT WINAPI dictionary_Keys(IDictionary *iface, VARIANT *keys) diff --git a/dlls/scrrun/tests/dictionary.c b/dlls/scrrun/tests/dictionary.c index 456a05ca7d2..0dfdb3bb2bd 100644 --- a/dlls/scrrun/tests/dictionary.c +++ b/dlls/scrrun/tests/dictionary.c @@ -1139,8 +1139,8 @@ static void test_put_Key(void) ok(count == 3, "Unexpected count %ld.\n", count); check_key_at(dict, 0, L"a", FALSE); - check_key_at(dict, 1, L"x", TRUE); - check_key_at(dict, 2, L"c", TRUE); + check_key_at(dict, 1, L"x", FALSE); + check_key_at(dict, 2, L"c", FALSE); /* Renaming a key that does not exist fails and changes nothing. */ V_VT(&key) = VT_BSTR; @@ -1148,15 +1148,15 @@ static void test_put_Key(void) V_VT(&newkey) = VT_BSTR; V_BSTR(&newkey) = SysAllocString(L"y"); hr = IDictionary_put_Key(dict, &key, &newkey); - todo_wine ok(hr == CTL_E_ELEMENT_NOT_FOUND, "put_Key of missing key: %#lx.\n", hr); + ok(hr == CTL_E_ELEMENT_NOT_FOUND, "put_Key of missing key: %#lx.\n", hr); VariantClear(&key); VariantClear(&newkey); hr = IDictionary_get_Count(dict, &count); ok(hr == S_OK, "get_Count: %#lx.\n", hr); - todo_wine ok(count == 3, "Unexpected count %ld.\n", count); + ok(count == 3, "Unexpected count %ld.\n", count); has_y = exists_str(dict, L"y"); - todo_wine ok(!has_y, "missing-key rename should not create 'y'\n"); + ok(!has_y, "missing-key rename should not create 'y'\n"); IDictionary_Release(dict); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10961