[PATCH v10 0/1] MR7052: vbscript: implement Join()
closes https://bugs.winehq.org/show_bug.cgi?id=56464 -- v10: vbscript: implement Join() https://gitlab.winehq.org/wine/wine/-/merge_requests/7052
From: Francis De Brabandere <francisdb(a)gmail.com> --- dlls/vbscript/global.c | 119 +++++++++++++++++++++++++++++++++++- dlls/vbscript/tests/api.vbs | 95 ++++++++++++++++++++++++++++ 2 files changed, 211 insertions(+), 3 deletions(-) diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c index 972d0d2ea0b..467f2fbaa6f 100644 --- a/dlls/vbscript/global.c +++ b/dlls/vbscript/global.c @@ -2581,10 +2581,123 @@ static HRESULT Global_Filter(BuiltinDisp *This, VARIANT *arg, unsigned args_cnt, return E_NOTIMPL; } -static HRESULT Global_Join(BuiltinDisp *This, VARIANT *arg, unsigned args_cnt, VARIANT *res) +static HRESULT Global_Join(BuiltinDisp *This, VARIANT *args, unsigned args_cnt, VARIANT *res) { - FIXME("\n"); - return E_NOTIMPL; + const WCHAR *delimiter = L" "; + BSTR free_delimiter = NULL, output = NULL, str = NULL; + SAFEARRAY *sa; + HRESULT hres; + LONG lbound, ubound; + VARIANT *data; + BSTR *str_array = NULL; + UINT total_len = 0, delimiter_len = 1, str_len; + WCHAR *output_ptr; + INT i; + + assert(1 <= args_cnt && args_cnt <= 2); + + switch(V_VT(args)) { + case VT_NULL: + return MAKE_VBSERROR(VBSE_ILLEGAL_NULL_USE); + case VT_DISPATCH: + return MAKE_VBSERROR(VBSE_OLE_NO_PROP_OR_METHOD); + case VT_VARIANT|VT_ARRAY: + sa = V_ARRAY(args); + break; + case VT_VARIANT|VT_ARRAY|VT_BYREF: + sa = *V_ARRAYREF(args); + break; + default: + return MAKE_VBSERROR(VBSE_TYPE_MISMATCH); + } + + if (args_cnt == 2) { + if (V_VT(args + 1) == VT_NULL) + return MAKE_VBSERROR(VBSE_ILLEGAL_NULL_USE); + if (V_VT(args + 1) != VT_BSTR) { + hres = to_string(args + 1, &free_delimiter); + if (FAILED(hres)) + return hres; + delimiter = free_delimiter; + delimiter_len = SysStringLen(free_delimiter); + } else { + delimiter = V_BSTR(args + 1); + } + } + + if (SafeArrayGetDim(sa) != 1) { + hres = MAKE_VBSERROR(VBSE_TYPE_MISMATCH); + goto cleanup; + } + + hres = SafeArrayGetLBound(sa, 1, &lbound); + if (FAILED(hres)) + goto cleanup; + + hres = SafeArrayGetUBound(sa, 1, &ubound); + if (FAILED(hres)) + goto cleanup; + + hres = SafeArrayAccessData(sa, (void**)&data); + if (FAILED(hres)) + goto cleanup; + + str_array = (BSTR *)malloc((ubound - lbound + 1) * sizeof(BSTR)); + if (!str_array) { + hres = E_OUTOFMEMORY; + goto cleanup_data; + } + + for (i = lbound; i <= ubound; i++) { + if (V_VT(&data[i]) != VT_BSTR) { + hres = to_string(&data[i], &str); + if (FAILED(hres)) + goto cleanup_data; + } else { + str = V_BSTR(&data[i]); + } + + str_array[i - lbound] = str; + total_len += SysStringLen(str); + if (i > lbound) + total_len += delimiter_len; + } + + output = SysAllocStringLen(NULL, total_len); + if (!output) { + hres = E_OUTOFMEMORY; + goto cleanup_data; + } + + output_ptr = output; + + for (i = lbound; i <= ubound; i++) { + if (i > lbound) { + memcpy(output_ptr, delimiter, delimiter_len * sizeof(WCHAR)); + output_ptr += delimiter_len; + } + str = str_array[i - lbound]; + str_len = SysStringLen(str); + memcpy(output_ptr, str, str_len * sizeof(WCHAR)); + output_ptr += str_len; + + if (V_VT(&data[i]) != VT_BSTR) + SysFreeString(str); + } + + *output_ptr = L'\0'; + SafeArrayUnaccessData(sa); + SysFreeString(free_delimiter); + free(str_array); + + return return_bstr(res, output); + +cleanup_data: + SafeArrayUnaccessData(sa); +cleanup: + SysFreeString(free_delimiter); + free(str_array); + return hres; } static HRESULT Global_Split(BuiltinDisp *This, VARIANT *args, unsigned args_cnt, VARIANT *res) diff --git a/dlls/vbscript/tests/api.vbs b/dlls/vbscript/tests/api.vbs index dfa2816bec5..7a27912279a 100644 --- a/dlls/vbscript/tests/api.vbs +++ b/dlls/vbscript/tests/api.vbs @@ -22,6 +22,45 @@ Dim x Class EmptyClass End Class +' Returns the amount of dimensions of an array. +' Returns 0 when it is not an array +Function GetDimensions(arr) + Dim dimension, upperBound + On error resume next + For dimension = 1 to 255 + upperBound = ubound(arr, dimension) + If err.Number <> 0 Then Exit for + Next + On error goto 0 + GetDimensions = dimension-1 +End Function + +' Helper function to print a variable +Function ToString(x) + If IsEmpty(x) Then + ToString = "Empty" + ElseIf IsNull(x) Then + ToString = "Null" + ElseIf IsObject(x) Then + ToString = "Object" + ElseIf IsArray(x) Then + Dim i, arrStr + arrStr = "Array(" + If GetDimensions(x) = 1 Then + For i = LBound(x) To UBound(x) + arrStr = arrStr & ToString(x(i)) + If i < UBound(x) Then arrStr = arrStr & ", " + Next + Else + arrStr = arrStr & "...multidim..." + End If + arrStr = arrStr & ")" + ToString = arrStr + Else + ToString = CStr(x) + End If +End Function + Call ok(vbSunday = 1, "vbSunday = " & vbSunday) Call ok(getVT(vbSunday) = "VT_I2", "getVT(vbSunday) = " & getVT(vbSunday)) Call ok(vbMonday = 2, "vbMonday = " & vbMonday) @@ -713,6 +752,62 @@ TestLCase 0.123, doubleAsString(0.123) TestLCase Empty, "" Call ok(getVT(LCase(Null)) = "VT_NULL", "getVT(LCase(Null)) = " & getVT(LCase(Null))) +' Join + +Sub TestJoin(arg, ex) + x = Join(arg) + Call ok(x = ex, "Join(" & ToString(arg) & ") = " & x & " expected " & ex) +End Sub + +Sub TestJoin2(arg1, arg2, ex) + x = Join(arg1, arg2) + Call ok(x = ex, "Join(" & ToString(arg1) & "," & arg2 & ") = " & x & " expected " & ex) +End Sub + +Sub TestJoinError(arg, num) + On Error Resume Next + Call Join(arg) + Dim err_num: err_num = Err.number + Call Err.clear() + On Error GoTo 0 + Call ok(err_num = num, "Join(" & ToString(arg) & ") expected Err.number = " & num & " got " & err_num) +End Sub + +TestJoin Array(), "" +TestJoin Array("a", "b", "c"), "a b c" +TestJoin Array("a", "b", "c", 1, 2, 3), "a b c 1 2 3" +TestJoin Array(1, Empty), "1 " + +TestJoin2 Array(), "", "" +TestJoin2 Array("a"), "-", "a" +TestJoin2 Array("a", "b"), "-", "a-b" +TestJoin2 Array("a", "b", "c"), "", "abc" +TestJoin2 Array(1, "Hello"), "-", "1-Hello" +TestJoin2 Array("a", "b", "c"), Empty, "abc" + +TestJoinError Null , 94 +TestJoinError Empty, 13 +TestJoinError 1, 13 +TestJoinError "test", 13 +TestJoinError 1.2, 13 +TestJoinError New EmptyClass, 438 +TestJoinError Array(1, Null), 13 +TestJoinError Array(Array(1, 2), Array(3, 4)), 13 +Dim multidim(2, 2) +TestJoinError multidim, 13 + +On Error Resume Next +Call Join(Array(), Null) +Call ok(Err.number = 94, "Join(Array(), Null) expected Err.number = 94 got " & Err.number) +Call Err.clear +On Error GoTo 0 + +On Error Resume Next +Call Join(Array(), "a", "b") +Call ok(Err.number = 450, "Join(Array(), ""a"", ""b"") expected Err.number = 450 got " & Err.number) +Call Err.clear +On Error GoTo 0 + x=Split("abc") Call ok(x(0) = "abc", "Split(""abc"")(0)=" & x(0)) x = Split("abc def") -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7052
Jacek Caban (@jacek) commented about dlls/vbscript/global.c:
+ + hres = SafeArrayAccessData(sa, (void**)&data); + if (FAILED(hres)) + goto cleanup; + + str_array = (BSTR *)malloc((ubound - lbound + 1) * sizeof(BSTR)); + if (!str_array) { + hres = E_OUTOFMEMORY; + goto cleanup_data; + } + + for (i = lbound; i <= ubound; i++) { + if (V_VT(&data[i]) != VT_BSTR) { + hres = to_string(&data[i], &str); + if (FAILED(hres)) + goto cleanup_data; In case of error, we'd leak previously converted strings, so cleanup code path should probably also iterate `str_array` and free them. We could move `SysFreeString` call to the cleanup part and have a separate loop there. With more code in the cleanup path, it would be tempting to share more with the success path: you could free everything first and then either return `hres` or `return_bstr` depending on `hres` variable.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7052#note_93563
Jacek Caban (@jacek) commented about dlls/vbscript/global.c:
+ break; + default: + return MAKE_VBSERROR(VBSE_TYPE_MISMATCH); + } + + if (args_cnt == 2) { + if (V_VT(args + 1) == VT_NULL) + return MAKE_VBSERROR(VBSE_ILLEGAL_NULL_USE); + if (V_VT(args + 1) != VT_BSTR) { + hres = to_string(args + 1, &free_delimiter); + if (FAILED(hres)) + return hres; + delimiter = free_delimiter; + delimiter_len = SysStringLen(free_delimiter); + } else { + delimiter = V_BSTR(args + 1); I think we should set `delimiter_len` here as well.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7052#note_93562
The approach looks good to me now, I just have some comments to error handling. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7052#note_93564
participants (3)
-
Francis De Brabandere -
Francis De Brabandere (＠francisdb)
-
Jacek Caban (＠jacek)