<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<div class="moz-cite-prefix">Hi Robert,</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">Thanks for the patch, I have a few
comments bellow.<br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">On 16.08.2020 23:55, Robert Wilhelm
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:c9e86e5ad2c7e31521d656418730bd76bf3fa21e.camel@gmx.net">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre class="moz-quote-pre" wrap="">Wine-Bug: <a class="moz-txt-link-freetext" href="https://bugs.winehq.org/show_bug.cgi?id=47570" moz-do-not-send="true">https://bugs.winehq.org/show_bug.cgi?id=47570</a>
Signed-off-by: Robert Wilhelm <a class="moz-txt-link-rfc2396E" href="mailto:robert.wilhelm@gmx.net" moz-do-not-send="true"><robert.wilhelm@gmx.net></a>
---
dlls/vbscript/global.c | 119 +++++++++++++++++++++++++++++++++++-
dlls/vbscript/tests/api.vbs | 19 ++++++
2 files changed, 135 insertions(+), 3 deletions(-)
diff --git a/dlls/vbscript/global.c b/dlls/vbscript/global.c
index 122f5c7b71..b799b962fe 100644
--- a/dlls/vbscript/global.c
+++ b/dlls/vbscript/global.c
@@ -2291,10 +2291,123 @@ static HRESULT Global_Join(BuiltinDisp *This, VARIANT *arg, unsigned args_cnt, V
return E_NOTIMPL;
}
-static HRESULT Global_Split(BuiltinDisp *This, VARIANT *arg, unsigned args_cnt, VARIANT *res)
+static HRESULT Global_Split(BuiltinDisp *This, VARIANT *args, unsigned args_cnt, VARIANT *res)
{
- FIXME("\n");
- return E_NOTIMPL;
+ BSTR str,string,delimeter;
+ int count,max,mode,len,start,end,ret;
+ SAFEARRAYBOUND bounds;
+ SAFEARRAY *sa;
+ VARIANT *data,var;
+ HRESULT hres = S_OK;
+
+ TRACE("%s %u...\n", debugstr_variant(args), args_cnt);
+
+ assert(1 <= args_cnt && args_cnt <= 4);
+
+ if(V_VT(args) != VT_BSTR) {
+ hres = to_string(args, &string);</pre>
</div>
</blockquote>
<p><br>
</p>
<p>string is never freed.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:c9e86e5ad2c7e31521d656418730bd76bf3fa21e.camel@gmx.net">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre class="moz-quote-pre" wrap="">
+ if(FAILED(hres))
+ return hres;
+ }else {
+ string = V_BSTR(args);
+ }
+
+ if ( args_cnt > 1)
+ {
+ if(V_VT(args+1) != VT_BSTR) {
+ hres = to_string(args+1, &delimeter);</pre>
</div>
</blockquote>
<p><br>
</p>
<p>delimiter is never freed.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:c9e86e5ad2c7e31521d656418730bd76bf3fa21e.camel@gmx.net">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre class="moz-quote-pre" wrap="">
+ if(FAILED(hres))
+ return hres;
+ }else {
+ delimeter = V_BSTR(args+1);
+ }
+ }
+ else
+ delimeter = SysAllocString(L" ");</pre>
</div>
</blockquote>
<p><br>
</p>
<p>In this case, we don't really need to allocate it. I'd suggest to
change how delimiter is handled to not require BSTR in this case.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:c9e86e5ad2c7e31521d656418730bd76bf3fa21e.camel@gmx.net">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre class="moz-quote-pre" wrap="">
+
+ if(args_cnt > 2) {
+ hres = to_int(args + 2, &max);
+ if(FAILED(hres))
+ return hres;
+ }else {
+ max = -1;
+ }
+
+ if (args_cnt == 4) {
+ hres = to_int(args+3, &mode);
+ if(FAILED(hres))
+ return hres;
+
+ if (mode != 0 && mode != 1) {
+ FIXME("unknown compare mode = %d\n", mode);
+ return E_FAIL;
+ }
+ }
+ else
+ mode = 0;
+
+ start = 0;
+
+ bounds.lLbound = 0;
+ bounds.cElements = 0;
+
+ sa = SafeArrayCreate( VT_VARIANT, 1, &bounds);
+ if (!sa)
+ return E_OUTOFMEMORY;</pre>
</div>
</blockquote>
<p><br>
</p>
<p>This is leaked in error code paths.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:c9e86e5ad2c7e31521d656418730bd76bf3fa21e.camel@gmx.net">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre class="moz-quote-pre" wrap="">
+
+ len = SysStringLen(string);
+ count = 0;
+
+ while(1)
+ {
+ ret = FindStringOrdinal(FIND_FROMSTART, string + start, len -start,
+ delimeter, SysStringLen(delimeter), mode);
+
+ if (ret == -1)
+ end = len;
+ else
+ end = start + ret;
+
+ str = SysAllocStringLen(string + start, end - start);
+ if (!str)
+ return E_OUTOFMEMORY;
+
+ V_VT(&var) = VT_BSTR;
+ V_BSTR(&var) = str;
+
+ bounds.cElements++;
+ SafeArrayRedim(sa, &bounds);
+ hres = SafeArrayAccessData(sa, (void**)&data);
+ if(FAILED(hres)) {
+ SafeArrayDestroy(sa);
+ return hres;
+ }
+
+ hres = VariantCopyInd(data+count, &var);
+
+ if(FAILED(hres)) {
+ SafeArrayUnaccessData(sa);
+ SafeArrayDestroy(sa);
+ return hres;
+ }
+
+ SafeArrayUnaccessData(sa);
+
+ if (ret == -1) break;
+
+ start = start + ret + SysStringLen(delimeter);
+ count++;
+ if (count == max) break;
+ }
+
+ if(res) {
+ V_VT(res) = VT_ARRAY|VT_VARIANT;
+ V_ARRAY(res) = sa;
+ }else {
+ SafeArrayDestroy(sa);
+ }
+
+ return S_OK;
}</pre>
</div>
</blockquote>
<p><br>
</p>
<p>Also please try to use consistent white space style. The patch
uses a few inconsistently, ideally it would use similar to
existing code in the file.<br>
</p>
<p><br>
</p>
<blockquote type="cite"
cite="mid:c9e86e5ad2c7e31521d656418730bd76bf3fa21e.camel@gmx.net">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 12px;"
lang="x-unicode">
<pre class="moz-quote-pre" wrap=""> 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..fe4799b52f 100644
--- a/dlls/vbscript/tests/api.vbs
+++ b/dlls/vbscript/tests/api.vbs
@@ -609,6 +609,25 @@ 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 returned " & x(0))
+x = Split("abc def")
+Call ok(x(0) = "abc", "Split returned " & x(0))
+Call ok(x(1) = "def", "Split returned " & x(1))
+x = Split("abc-def","-")
+Call ok(x(0) = "abc", "Split returned " & x(0))
+Call ok(x(1) = "def", "Split returned " & x(1))
+x = Split("abc-def-ghi","-")
+Call ok(UBound(x) = 2, "Split returned " & UBound(x))
+x = Split("abc-def-ghi","-",2)
+Call ok(UBound(x) = 1, "Split returned " & UBound(x))
+x = Split("abcZdefZghi","Z",3,0)
+Call ok(UBound(x) = 2, "Split returned " & UBound(x))
+x = Split("abcZdefZghi","z",3,0)
+Call ok(UBound(x) = 0, "Split returned " & UBound(x))
+x = Split("abcZdefZghi","z",3,1)
+Call ok(UBound(x) = 2, "Split returned " & UBound(x))
+</pre>
</div>
</blockquote>
<p><br>
</p>
<p>It would be interesting to see a bit more tests. For example,
empty ("") delimiter, delimiter with more than one character, a
case that returns more than 2 elements.</p>
<p><br>
</p>
<p>Thanks,</p>
<p>Jacek<br>
</p>
</body>
</html>