From: Francis De Brabandere <francisdb@gmail.com> get_keyitem_pair() returned NULL both when a key was absent and when its hash could not be computed, so Exists reported a present-but-unhashable object key as absent and Remove failed with element-not-found instead of the hashing error. Return the HashVal failure to callers so Exists, Remove, Item, Add, and Key all surface CTL_E_ILLEGALFUNCTIONCALL for a degenerate object key, matching native. --- dlls/scrrun/dictionary.c | 59 ++++++++++++++++++++++++++-------- dlls/scrrun/tests/dictionary.c | 4 +-- 2 files changed, 47 insertions(+), 16 deletions(-) diff --git a/dlls/scrrun/dictionary.c b/dlls/scrrun/dictionary.c index af771c9245a..1b618865360 100644 --- a/dlls/scrrun/dictionary.c +++ b/dlls/scrrun/dictionary.c @@ -199,24 +199,30 @@ static BOOL is_matching_key(const struct dictionary *dict, const struct keyitem_ } } -static struct keyitem_pair *get_keyitem_pair(struct dictionary *dict, VARIANT *key) +/* Looks up the pair for a key. On success *ret is the matching pair, or NULL + if the key is not present. A failure to hash the key (e.g. an object key + that can no longer be queried for IID_IUnknown) is reported to the caller + rather than being mistaken for an absent key. */ +static HRESULT get_keyitem_pair(struct dictionary *dict, VARIANT *key, struct keyitem_pair **ret) { struct keyitem_pair *pair; struct list *head, *entry; VARIANT hash, v; HRESULT hr; + *ret = NULL; + hr = IDictionary_get_HashVal(&dict->IDictionary_iface, key, &hash); if (FAILED(hr)) - return NULL; + return hr; head = get_bucket_head(dict, V_I4(&hash)); if (!head->next || list_empty(head)) - return NULL; + return S_OK; VariantInit(&v); - if (FAILED(VariantCopyInd(&v, key))) - return NULL; + if (FAILED(hr = VariantCopyInd(&v, key))) + return hr; entry = list_head(head); do @@ -225,13 +231,14 @@ static struct keyitem_pair *get_keyitem_pair(struct dictionary *dict, VARIANT *k if (is_matching_key(dict, pair, &v, V_I4(&hash))) { VariantClear(&v); - return pair; + *ret = pair; + return S_OK; } } while ((entry = list_next(head, entry))); VariantClear(&v); - return NULL; + return S_OK; } static HRESULT add_keyitem_pair(struct dictionary *dict, VARIANT *key, VARIANT *item) @@ -563,10 +570,13 @@ static HRESULT WINAPI dictionary_putref_Item(IDictionary *iface, VARIANT *key, V { struct dictionary *dictionary = impl_from_IDictionary(iface); struct keyitem_pair *pair; + HRESULT hr; TRACE("%p, %s, %s.\n", iface, debugstr_variant(key), debugstr_variant(item)); - if ((pair = get_keyitem_pair(dictionary, key))) + if (FAILED(hr = get_keyitem_pair(dictionary, key, &pair))) + return hr; + if (pair) return VariantCopyInd(&pair->item, item); return add_keyitem_pair(dictionary, key, item); @@ -576,10 +586,13 @@ static HRESULT WINAPI dictionary_put_Item(IDictionary *iface, VARIANT *key, VARI { struct dictionary *dictionary = impl_from_IDictionary(iface); struct keyitem_pair *pair; + HRESULT hr; TRACE("%p, %s, %s.\n", iface, debugstr_variant(key), debugstr_variant(item)); - if ((pair = get_keyitem_pair(dictionary, key))) + if (FAILED(hr = get_keyitem_pair(dictionary, key, &pair))) + return hr; + if (pair) return VariantCopyInd(&pair->item, item); return add_keyitem_pair(dictionary, key, item); @@ -589,10 +602,13 @@ static HRESULT WINAPI dictionary_get_Item(IDictionary *iface, VARIANT *key, VARI { struct dictionary *dictionary = impl_from_IDictionary(iface); struct keyitem_pair *pair; + HRESULT hr; TRACE("%p, %s, %p.\n", iface, debugstr_variant(key), item); - if ((pair = get_keyitem_pair(dictionary, key))) + if (FAILED(hr = get_keyitem_pair(dictionary, key, &pair))) + return hr; + if (pair) VariantCopy(item, &pair->item); else { VariantInit(item); @@ -605,10 +621,14 @@ static HRESULT WINAPI dictionary_get_Item(IDictionary *iface, VARIANT *key, VARI static HRESULT WINAPI dictionary_Add(IDictionary *iface, VARIANT *key, VARIANT *item) { struct dictionary *dictionary = impl_from_IDictionary(iface); + struct keyitem_pair *pair; + HRESULT hr; TRACE("%p, %s, %s.\n", iface, debugstr_variant(key), debugstr_variant(item)); - if (get_keyitem_pair(dictionary, key)) + if (FAILED(hr = get_keyitem_pair(dictionary, key, &pair))) + return hr; + if (pair) return CTL_E_KEY_ALREADY_EXISTS; return add_keyitem_pair(dictionary, key, item); @@ -627,13 +647,18 @@ static HRESULT WINAPI dictionary_get_Count(IDictionary *iface, LONG *count) static HRESULT WINAPI dictionary_Exists(IDictionary *iface, VARIANT *key, VARIANT_BOOL *exists) { struct dictionary *dictionary = impl_from_IDictionary(iface); + struct keyitem_pair *pair; + HRESULT hr; TRACE("%p, %s, %p.\n", iface, debugstr_variant(key), exists); if (!exists) return CTL_E_ILLEGALFUNCTIONCALL; - *exists = get_keyitem_pair(dictionary, key) != NULL ? VARIANT_TRUE : VARIANT_FALSE; + if (FAILED(hr = get_keyitem_pair(dictionary, key, &pair))) + return hr; + + *exists = pair ? VARIANT_TRUE : VARIANT_FALSE; return S_OK; } @@ -686,7 +711,10 @@ static HRESULT WINAPI dictionary_put_Key(IDictionary *iface, VARIANT *key, VARIA TRACE("%p, %s, %s.\n", iface, debugstr_variant(key), debugstr_variant(newkey)); - if ((pair = get_keyitem_pair(dictionary, key))) + if (FAILED(hr = get_keyitem_pair(dictionary, key, &pair))) + return hr; + + if (pair) { /* found existing pair for a key, add new pair with new key and old item and remove old pair after that */ @@ -746,10 +774,13 @@ static HRESULT WINAPI dictionary_Remove(IDictionary *iface, VARIANT *key) { struct dictionary *dictionary = impl_from_IDictionary(iface); struct keyitem_pair *pair; + HRESULT hr; TRACE("%p, %s.\n", iface, debugstr_variant(key)); - if (!(pair = get_keyitem_pair(dictionary, key))) + if (FAILED(hr = get_keyitem_pair(dictionary, key, &pair))) + return hr; + if (!pair) return CTL_E_ELEMENT_NOT_FOUND; notify_remove_pair(&dictionary->notifier, &pair->entry); diff --git a/dlls/scrrun/tests/dictionary.c b/dlls/scrrun/tests/dictionary.c index 7fbaaa91001..8fc369e7062 100644 --- a/dlls/scrrun/tests/dictionary.c +++ b/dlls/scrrun/tests/dictionary.c @@ -397,10 +397,10 @@ static void test_object_key_hashfail(void) obj.qi_unknown_fails = TRUE; hr = IDictionary_Exists(dict, &key, &exists); - todo_wine ok(hr == CTL_E_ILLEGALFUNCTIONCALL, "Exists (degenerate): %#lx.\n", hr); + ok(hr == CTL_E_ILLEGALFUNCTIONCALL, "Exists (degenerate): %#lx.\n", hr); hr = IDictionary_Remove(dict, &key); - todo_wine ok(hr == CTL_E_ILLEGALFUNCTIONCALL, "Remove (degenerate): %#lx.\n", hr); + ok(hr == CTL_E_ILLEGALFUNCTIONCALL, "Remove (degenerate): %#lx.\n", hr); /* Restore the object and confirm the key is still present and removable. */ obj.qi_unknown_fails = FALSE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10978