[PATCH v4] vbscript: Implement Split.

[Jacek Caban]

Hi Robert,

The patch looks better now, thanks.

On 19.08.2020 22:34, Robert Wilhelm wrote:
> +        V_VT(&var) = VT_BSTR;
> +        V_BSTR(&var) = str;
> +        bounds.cElements++;
> +        SafeArrayRedim(sa, &bounds);
> +        hres = SafeArrayAccessData(sa, (void**)&data);
> +        if(FAILED(hres)) {
> +            SafeArrayDestroy(sa);
> +            break;
> +        }


Resizing safe array in each iteration is a bit suboptimal. Maybe we 
could have a temporary array of result strings (that we could grow 
exponentially) and construct the actual SAFEARRAY when we know the size.


> +
> +        hres = VariantCopyInd(data+count, &var);
> +        if(FAILED(hres)) {
> +            SafeArrayUnaccessData(sa);
> +            SafeArrayDestroy(sa);
> +            break;
> +        }
> +
> +        SafeArrayUnaccessData(sa);
> +        count++;
> +
> +        if (ret == -1) break;
> +        start = start + ret + delimeterlen;
> +        if (count == max) break;
> +        if (start > len) break;
> +    }
> +
> +error:
> +    if(res) {
> +        V_VT(res) = VT_ARRAY|VT_VARIANT;
> +        V_ARRAY(res) = sa;


We should set res value only when we return success. Otherwise the 
caller may assume that data is invalid and leak it.


> +    }else {
> +        if (sa) SafeArrayDestroy(sa);
> +    }
> +
> +    if(V_VT(args) != VT_BSTR)
> +        SysFreeString(string);
> +    if(V_VT(args+1) != VT_BSTR)
> +        SysFreeString(delimeter);
> +    return hres;
>   }
>
>   static HRESULT Global_Replace(BuiltinDisp *This, VARIANT *args, unsigned args_cnt, VARIANT *res)
> diff --git a/dlls/vbscript/tests/api.vbs b/dlls/vbscript/tests/api.vbs
> index 6af49c849d..60ad6359dc 100644
> --- a/dlls/vbscript/tests/api.vbs
> +++ b/dlls/vbscript/tests/api.vbs
> @@ -609,6 +609,49 @@ TestLCase 0.123, doubleAsString(0.123)
>   TestLCase Empty, ""
>   Call ok(getVT(LCase(Null)) = "VT_NULL", "getVT(LCase(Null)) = " & getVT(LCase(Null)))
>
> +x=Split("abc")
> +Call ok(x(0) = "abc", "Split(""abc"")(0)=" & x(0))
> +x = Split("abc def")
> +Call ok(x(0) = "abc", "Split(""abc def"")(0)=" & x(0))
> +Call ok(x(1) = "def", "Split(""abc def"")(1)=" & x(1))
> +x = Split("abc def ghi")
> +Call ok(x(0) = "abc", "Split(""abc def ghi"")(0)=" & x(0))
> +Call ok(x(1) = "def", "Split(""abc def ghi"")(1)=" & x(1))
> +Call ok(x(2) = "ghi", "Split(""abc def ghi"")(2)=" & x(2))
> +x = Split("abc def","")
> +Call ok(x(0) = "abc def", "Split(""abc def"","""")(0)=" & x(0))
> +x = Split("abc-def","-")
> +Call ok(x(0) = "abc", "Split(""abc-def"",""-"")(0)=" & x(0))
> +Call ok(x(1) = "def", "Split(""abc-def"",""-"")(1)=" & x(1))
> +x = Split("abc--def","-")
> +Call ok(x(0) = "abc", "Split(""abc--def"",""-"")(0)=" & x(0))
> +Call ok(x(1) = "",    "Split(""abc--def"",""-"")(1)=" & x(1))
> +Call ok(x(2) = "def", "Split(""abc--def"",""-"")(2)=" & x(2))
> +x = Split("abcdefghi","def")
> +Call ok(x(0) = "abc", "Split(""abcdefghi"",""def"")(0)=" & x(0))
> +Call ok(x(1) = "ghi", "Split(""abcdefghi"",""def"")(1)=" & x(1))
> +x = Split("12345",3)
> +Call ok(x(0) = "12", "Split(""12345"",3)(0)=" & x(0))
> +Call ok(x(1) = "45", "Split(""12345"",3)(1)=" & x(1))
> +x = Split("12345",5)
> +Call ok(x(0) = "1234", "Split(""12345"",5)(0)=" & x(0))
> +Call ok(x(1) = "",     "Split(""12345"",5)(1)=" & x(1))
> +x = Split("12345",12)
> +Call ok(x(0) = "", "Split(""12345"",12)(0)=" & x(0))
> +Call ok(x(1) = "345", "Split(""12345"",12)(1)=" & x(1))
> +x = Split("abc-def-ghi","-")
> +Call ok(UBound(x) = 2, "UBound(Split(""abc-def-ghi"",""-""))=" & UBound(x))
> +x = Split("abc-def-ghi","-",2)
> +Call ok(UBound(x) = 1, "UBound(Split(""abc-def-ghi"",""-"",2))=" & UBound(x))
> +x = Split("abc-def-ghi","-",4)
> +Call ok(UBound(x) = 2, "UBound(Split(""abc-def-ghi"",""-"",4))="  & UBound(x))
> +x = Split("abcZdefZghi","Z",3,0)
> +Call ok(UBound(x) = 2, "UBound(Split(""abcZdefZghi"",""Z"",3,0))="  & UBound(x))
> +x = Split("abcZdefZghi","z",3,0)
> +Call ok(UBound(x) = 0, "UBound(Split(""abcZdefZghi"",""z"",3,0))=" & UBound(x))
> +x = Split("abcZdefZghi","z",3,1)
> +Call ok(UBound(x) = 2, "UBound(Split(""abcZdefZghi"",""z"",3,1))=" & UBound(x))


A test for negative "max" argument would be nice: both an explicit -1 
and some other value.


Thanks,

Jacek