[PATCH v2 0/2] MR10960: scrrun/dictionary: Match Empty keys against zero-valued and empty-string keys.
An uninitialized VBScript variable is Empty, so scripts can end up using Empty as a dictionary key alongside a numeric 0 or "". Native treats those as the same key; Wine kept Empty distinct from every other type. Match an Empty key against any numeric zero or the empty string, while keeping it distinct from Null and from non-zero/non-empty keys. -- v2: scrrun/dictionary: Match Empty keys against zero-valued and empty-string keys. scrrun/tests: Add tests for Empty dictionary keys. https://gitlab.winehq.org/wine/wine/-/merge_requests/10960
From: Francis De Brabandere <francisdb@gmail.com> An Empty key matches the zero/default value of the other key's type: any numeric zero or the empty string, but not a non-zero number, a non-empty string, or Null. --- dlls/scrrun/tests/dictionary.c | 82 ++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/dlls/scrrun/tests/dictionary.c b/dlls/scrrun/tests/dictionary.c index e1cb33c45a7..9e526845fc0 100644 --- a/dlls/scrrun/tests/dictionary.c +++ b/dlls/scrrun/tests/dictionary.c @@ -1050,6 +1050,87 @@ static void test_Add(void) IDictionary_Release(dict); } +/* Add two keys to a fresh dictionary; returns TRUE if the second key is + * treated as a duplicate of the first. */ +static BOOL keys_match(VARIANT *key1, VARIANT *key2) +{ + IDictionary *dict; + VARIANT item; + HRESULT hr; + + if (FAILED(CoCreateInstance(&CLSID_Dictionary, NULL, CLSCTX_INPROC_SERVER|CLSCTX_INPROC_HANDLER, + &IID_IDictionary, (void**)&dict))) + return FALSE; + + V_VT(&item) = VT_I2; + V_I2(&item) = 1; + if (FAILED(IDictionary_Add(dict, key1, &item))) + { + IDictionary_Release(dict); + return FALSE; + } + + hr = IDictionary_Add(dict, key2, &item); + IDictionary_Release(dict); + return hr == CTL_E_KEY_ALREADY_EXISTS; +} + +static void test_empty_key(void) +{ + static const VARTYPE zero_keys[] = { + VT_I2, VT_I4, VT_UI1, VT_R4, VT_R8, VT_DATE, + }; + VARIANT a, b; + unsigned i; + + /* Empty matches the zero/default value of any numeric type. */ + for (i = 0; i < ARRAY_SIZE(zero_keys); i++) + { + V_VT(&a) = VT_EMPTY; + V_VT(&b) = zero_keys[i]; + if (zero_keys[i] == VT_R8 || zero_keys[i] == VT_DATE) + V_R8(&b) = 0.0; + else if (zero_keys[i] == VT_R4) + V_R4(&b) = 0.0f; + else + V_I4(&b) = 0; + todo_wine ok(keys_match(&a, &b), "Empty should match zero key vt %d\n", zero_keys[i]); + } + + /* Empty matches the empty string, but not a non-empty one. */ + V_VT(&a) = VT_EMPTY; + V_VT(&b) = VT_BSTR; + V_BSTR(&b) = SysAllocString(L""); + todo_wine ok(keys_match(&a, &b), "Empty should match empty string\n"); + VariantClear(&b); + + V_VT(&b) = VT_BSTR; + V_BSTR(&b) = SysAllocString(L"0"); + ok(!keys_match(&a, &b), "Empty should not match \"0\"\n"); + VariantClear(&b); + + /* Empty does not match a non-zero number or Null. */ + V_VT(&b) = VT_I2; + V_I2(&b) = 5; + ok(!keys_match(&a, &b), "Empty should not match I2 5\n"); + + V_VT(&b) = VT_NULL; + ok(!keys_match(&a, &b), "Empty should not match Null\n"); + + /* A numeric zero does not match the empty string (only Empty bridges). */ + V_VT(&a) = VT_I2; + V_I2(&a) = 0; + V_VT(&b) = VT_BSTR; + V_BSTR(&b) = SysAllocString(L""); + ok(!keys_match(&a, &b), "I2 0 should not match empty string\n"); + VariantClear(&b); + + /* Null matches only Null. */ + V_VT(&a) = VT_NULL; + V_VT(&b) = VT_NULL; + ok(keys_match(&a, &b), "Null should match Null\n"); +} + static void test_IEnumVARIANT(void) { IUnknown *enum1, *enum2; @@ -1230,6 +1311,7 @@ START_TEST(dictionary) test_Remove(); test_Item(); test_Add(); + test_empty_key(); test_IEnumVARIANT(); test_putref_Item(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10960
From: Francis De Brabandere <francisdb@gmail.com> An uninitialized VBScript variable is Empty, so scripts can end up using Empty as a dictionary key alongside a numeric 0 or "". Native treats those as the same key; Wine kept Empty distinct from every other type. Match an Empty key against any numeric zero or the empty string, while keeping it distinct from Null and from non-zero/non-empty keys. --- dlls/scrrun/dictionary.c | 35 +++++++++++++++++++++++++++++++--- dlls/scrrun/tests/dictionary.c | 4 ++-- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/dlls/scrrun/dictionary.c b/dlls/scrrun/dictionary.c index af771c9245a..65ae2fee35b 100644 --- a/dlls/scrrun/dictionary.c +++ b/dlls/scrrun/dictionary.c @@ -162,9 +162,38 @@ static inline BOOL numeric_key_eq(const VARIANT *key1, const VARIANT *key2) return V_R4(&v1) == V_R4(&v2); } +/* Empty matches the zero/default value of the other key's type: any numeric + * zero, the empty string, or another Empty. */ +static BOOL empty_matches_key(const VARIANT *key) +{ + VARIANT v; + + switch (V_VT(key)) + { + case VT_EMPTY: + return TRUE; + case VT_BSTR: + { + const WCHAR *str = get_key_strptr(key); + return !str || !*str; + } + default: + if (!is_numeric_key(key)) + return FALSE; + VariantInit(&v); + if (FAILED(VariantChangeType(&v, key, 0, VT_R4))) + return FALSE; + return V_R4(&v) == 0.0f; + } +} + static BOOL is_matching_key(const struct dictionary *dict, const struct keyitem_pair *pair, const VARIANT *key, DWORD hash) { - if (is_string_key(key) != is_string_key(&pair->key)) + if (V_VT(key) == VT_EMPTY || V_VT(&pair->key) == VT_EMPTY) + { + return empty_matches_key(V_VT(key) == VT_EMPTY ? &pair->key : key); + } + else if (is_string_key(key) != is_string_key(&pair->key)) { return FALSE; } @@ -188,9 +217,9 @@ static BOOL is_matching_key(const struct dictionary *dict, const struct keyitem_ { return hash == pair->hash && numeric_key_eq(key, &pair->key); } - else if (V_VT(&pair->key) == VT_EMPTY || V_VT(&pair->key) == VT_NULL) + else if (V_VT(&pair->key) == VT_NULL) { - return V_VT(&pair->key) == V_VT(key); + return V_VT(key) == VT_NULL; } else { diff --git a/dlls/scrrun/tests/dictionary.c b/dlls/scrrun/tests/dictionary.c index 9e526845fc0..94d109987fc 100644 --- a/dlls/scrrun/tests/dictionary.c +++ b/dlls/scrrun/tests/dictionary.c @@ -1094,14 +1094,14 @@ static void test_empty_key(void) V_R4(&b) = 0.0f; else V_I4(&b) = 0; - todo_wine ok(keys_match(&a, &b), "Empty should match zero key vt %d\n", zero_keys[i]); + ok(keys_match(&a, &b), "Empty should match zero key vt %d\n", zero_keys[i]); } /* Empty matches the empty string, but not a non-empty one. */ V_VT(&a) = VT_EMPTY; V_VT(&b) = VT_BSTR; V_BSTR(&b) = SysAllocString(L""); - todo_wine ok(keys_match(&a, &b), "Empty should match empty string\n"); + ok(keys_match(&a, &b), "Empty should match empty string\n"); VariantClear(&b); V_VT(&b) = VT_BSTR; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10960
On Wed May 20 09:25:33 2026 +0000, Francis De Brabandere wrote:
No. Do you want me to remove the fixes for those? My workflow is: I find an issue (in vpinball scripts running using libwinevbs), test on windows, find more issues, fix all. Dropped: Boolean keys, Currency keys, and the R8-precision change
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10960#note_140769
On Wed May 20 09:25:33 2026 +0000, Francis De Brabandere wrote:
Dropped: Boolean keys, Currency keys, and the R8-precision change Yes, let's focus on things that you find in actual scripts. As long we have a clear fixme for unimplemented types, we'll spot them easily.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10960#note_140774
participants (3)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)
-
Nikolay Sivov (＠nsivov)