[PATCH v10 0/1] MR7052: vbscript: implement Join()

[Francis De Brabandere (＠francisdb)]

closes https://bugs.winehq.org/show_bug.cgi?id=56464

--
  v10: vbscript: implement Join()

https://gitlab.winehq.org/wine/wine/-/merge_requests/7052

[PATCH v10 1/1] vbscript: implement Join()

[Francis De Brabandere]

From: Francis De Brabandere

---
 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")

[Jacek Caban (＠jacek)]

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.

[Jacek Caban (＠jacek)]

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.

[Jacek Caban (＠jacek)]

The approach looks good to me now, I just have some comments to error handling.