[PATCH v4 2/2] scrrun/dictionary: Rename keys in place in put_Key.

[Francis De Brabandere]

From: Francis De Brabandere <[e-mail]>

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       | 56 ++++++++++++++++++++++------------
 dlls/scrrun/tests/dictionary.c | 10 +++---
 2 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/dlls/scrrun/dictionary.c b/dlls/scrrun/dictionary.c
index 1b618865360..f1bd646e29e 100644
--- a/dlls/scrrun/dictionary.c
+++ b/dlls/scrrun/dictionary.c
@@ -241,10 +241,19 @@ static HRESULT get_keyitem_pair(struct dictionary *dict, VARIANT *key, struct ke
     return S_OK;
 }
 
+static void link_keyitem_to_bucket(struct dictionary *dict, struct keyitem_pair *pair)
+{
+    struct list *head = get_bucket_head(dict, pair->hash);
+
+    if (!head->next)
+        /* this only happens once per bucket */
+        list_init(head);
+    list_add_tail(head, &pair->bucket);
+}
+
 static HRESULT add_keyitem_pair(struct dictionary *dict, VARIANT *key, VARIANT *item)
 {
     struct keyitem_pair *pair;
-    struct list *head;
     VARIANT hash;
     HRESULT hr;
 
@@ -267,13 +276,8 @@ static HRESULT add_keyitem_pair(struct dictionary *dict, VARIANT *key, VARIANT *
     if (FAILED(hr))
         goto failed;
 
-    head = get_bucket_head(dict, pair->hash);
-    if (!head->next)
-        /* this only happens once per bucket */
-        list_init(head);
-
     /* link to bucket list and to full list */
-    list_add_tail(head, &pair->bucket);
+    link_keyitem_to_bucket(dict, pair);
     list_add_tail(&dict->pairs, &pair->entry);
     dict->count++;
     return S_OK;
@@ -705,29 +709,41 @@ static HRESULT WINAPI dictionary_Items(IDictionary *iface, VARIANT *items)
 static HRESULT WINAPI dictionary_put_Key(IDictionary *iface, VARIANT *key, VARIANT *newkey)
 {
     struct dictionary *dictionary = impl_from_IDictionary(iface);
-    struct keyitem_pair *pair;
-    VARIANT empty;
+    struct keyitem_pair *pair, *existing;
+    VARIANT new_key, hash;
     HRESULT hr;
 
     TRACE("%p, %s, %s.\n", iface, debugstr_variant(key), debugstr_variant(newkey));
 
     if (FAILED(hr = get_keyitem_pair(dictionary, key, &pair)))
         return hr;
+    if (!pair)
+        return CTL_E_ELEMENT_NOT_FOUND;
 
-    if (pair)
-    {
-        /* found existing pair for a key, add new pair with new key
-           and old item and remove old pair after that */
+    /* The key is renamed in place, preserving its value and enumeration
+       position; only collisions with a different pair are rejected. */
+    if (FAILED(hr = get_keyitem_pair(dictionary, newkey, &existing)))
+        return hr;
+    if (existing && existing != pair)
+        return CTL_E_KEY_ALREADY_EXISTS;
 
-        hr = IDictionary_Add(iface, newkey, &pair->item);
-        if (FAILED(hr))
-            return hr;
+    hr = IDictionary_get_HashVal(iface, newkey, &hash);
+    if (FAILED(hr))
+        return hr;
 
-        return IDictionary_Remove(iface, key);
-    }
+    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);
 
-    VariantInit(&empty);
-    return IDictionary_Add(iface, newkey, &empty);
+    list_remove(&pair->bucket);
+    link_keyitem_to_bucket(dictionary, pair);
+
+    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 23b52a54dd3..3c052caed0e 100644
--- a/dlls/scrrun/tests/dictionary.c
+++ b/dlls/scrrun/tests/dictionary.c
@@ -1223,8 +1223,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;
@@ -1232,15 +1232,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