Signed-off-by: Dmitry Timoshkov dmitry@baikal.ru --- dlls/vbscript/vbregexp.c | 76 +++++++++++++--------------------------- 1 file changed, 25 insertions(+), 51 deletions(-)
diff --git a/dlls/vbscript/vbregexp.c b/dlls/vbscript/vbregexp.c index 4edf9dcbfe..50652da953 100644 --- a/dlls/vbscript/vbregexp.c +++ b/dlls/vbscript/vbregexp.c @@ -1433,45 +1433,14 @@ static HRESULT WINAPI RegExp2_Test(IRegExp2 *iface, BSTR sourceString, VARIANT_B return hres; }
-typedef struct { - WCHAR *buf; - DWORD size; - DWORD len; -} strbuf_t; - -static BOOL strbuf_ensure_size(strbuf_t *buf, unsigned len) -{ - WCHAR *new_buf; - DWORD new_size; - - if(len <= buf->size) - return TRUE; - - new_size = buf->size ? buf->size<<1 : 16; - if(new_size < len) - new_size = len; - if(buf->buf) - new_buf = heap_realloc(buf->buf, new_size*sizeof(WCHAR)); - else - new_buf = heap_alloc(new_size*sizeof(WCHAR)); - if(!new_buf) - return FALSE; - - buf->buf = new_buf; - buf->size = new_size; - return TRUE; -} - -static HRESULT strbuf_append(strbuf_t *buf, const WCHAR *str, DWORD len) +static HRESULT bstrcat(BSTR *str, const WCHAR *append, unsigned int len) { - if(!len) - return S_OK; + unsigned int old_len = SysStringLen(*str);
- if(!strbuf_ensure_size(buf, buf->len+len)) + if (!SysReAllocStringLen(str, NULL, old_len + len)) return E_OUTOFMEMORY;
- memcpy(buf->buf+buf->len, str, len*sizeof(WCHAR)); - buf->len += len; + memcpy(*str + old_len, append, len * sizeof(WCHAR)); return S_OK; }
@@ -1480,11 +1449,10 @@ static HRESULT WINAPI RegExp2_Replace(IRegExp2 *iface, BSTR source, VARIANT repl RegExp2 *This = impl_from_IRegExp2(iface); const WCHAR *cp, *prev_cp = NULL, *ptr, *prev_ptr; size_t match_len = 0, source_len, replace_len; - strbuf_t buf = { NULL, 0, 0 }; match_state_t *state = NULL; heap_pool_t *mark; VARIANT strv; - BSTR replace; + BSTR replace, buf; HRESULT hres;
TRACE("(%p)->(%s %s %p)\n", This, debugstr_w(source), debugstr_variant(&replaceVar), ret); @@ -1515,6 +1483,10 @@ static HRESULT WINAPI RegExp2_Replace(IRegExp2 *iface, BSTR source, VARIANT repl if(This->regexp && !(state = alloc_match_state(This->regexp, &This->pool, cp))) hres = E_OUTOFMEMORY;
+ buf = SysAllocString(L""); + if(!buf) + hres = E_OUTOFMEMORY; + while(SUCCEEDED(hres)) { if(This->regexp) { prev_cp = cp; @@ -1530,38 +1502,38 @@ static HRESULT WINAPI RegExp2_Replace(IRegExp2 *iface, BSTR source, VARIANT repl prev_cp = cp; }
- hres = strbuf_append(&buf, prev_cp, cp - prev_cp - match_len); + hres = bstrcat(&buf, prev_cp, cp - prev_cp - match_len); if(FAILED(hres)) break;
prev_ptr = replace; while((ptr = wmemchr(prev_ptr, '$', replace + replace_len - prev_ptr))) { - hres = strbuf_append(&buf, prev_ptr, ptr - prev_ptr); + hres = bstrcat(&buf, prev_ptr, ptr - prev_ptr); if(FAILED(hres)) break;
switch(ptr[1]) { case '$': - hres = strbuf_append(&buf, ptr, 1); + hres = bstrcat(&buf, ptr, 1); prev_ptr = ptr + 2; break; case '&': - hres = strbuf_append(&buf, cp - match_len, match_len); + hres = bstrcat(&buf, cp - match_len, match_len); prev_ptr = ptr + 2; break; case '`': - hres = strbuf_append(&buf, source, cp - source - match_len); + hres = bstrcat(&buf, source, cp - source - match_len); prev_ptr = ptr + 2; break; case ''': - hres = strbuf_append(&buf, cp, source + source_len - cp); + hres = bstrcat(&buf, cp, source + source_len - cp); prev_ptr = ptr + 2; break; default: { DWORD idx;
if(!iswdigit(ptr[1])) { - hres = strbuf_append(&buf, ptr, 1); + hres = bstrcat(&buf, ptr, 1); prev_ptr = ptr + 1; break; } @@ -1573,13 +1545,13 @@ static HRESULT WINAPI RegExp2_Replace(IRegExp2 *iface, BSTR source, VARIANT repl }else if(idx && idx <= state->paren_count) { prev_ptr = ptr + 2; }else { - hres = strbuf_append(&buf, ptr, 1); + hres = bstrcat(&buf, ptr, 1); prev_ptr = ptr + 1; break; }
if(state->parens[idx - 1].index != -1) - hres = strbuf_append(&buf, source + state->parens[idx - 1].index, + hres = bstrcat(&buf, source + state->parens[idx - 1].index, state->parens[idx - 1].length); break; } @@ -1588,7 +1560,7 @@ static HRESULT WINAPI RegExp2_Replace(IRegExp2 *iface, BSTR source, VARIANT repl break; } if(SUCCEEDED(hres)) - hres = strbuf_append(&buf, prev_ptr, replace + replace_len - prev_ptr); + hres = bstrcat(&buf, prev_ptr, replace + replace_len - prev_ptr); if(FAILED(hres)) break;
@@ -1597,13 +1569,15 @@ static HRESULT WINAPI RegExp2_Replace(IRegExp2 *iface, BSTR source, VARIANT repl }
if(SUCCEEDED(hres)) { - hres = strbuf_append(&buf, cp, source + source_len - cp); - if(SUCCEEDED(hres) && !(*ret = SysAllocStringLen(buf.buf, buf.len))) - hres = E_OUTOFMEMORY; + hres = bstrcat(&buf, cp, source + source_len - cp); + if(SUCCEEDED(hres)) + *ret = buf; }
+ if(FAILED(hres)) + SysFreeString(buf); + heap_pool_clear(mark); - heap_free(buf.buf); SysFreeString(replace); return hres; }
Hi Dmitry,
The problem with using BSTRs is that you need to realloc every time something is appended. In cases like [1], the current approach uses one heap_alloc and one SysAllocString. You patch would need one SysAllocString and 6 reallocation.
That said, I wouldn't mind your approach in general, but I don't think it's an improvement over the existing code.
Thanks,
Jacek
[1] https://source.winehq.org/git/wine.git/blob/HEAD:/dlls/vbscript/tests/regexp...
Jacek Caban jacek@codeweavers.com wrote:
The problem with using BSTRs is that you need to realloc every time something is appended. In cases like [1], the current approach uses one heap_alloc and one SysAllocString. You patch would need one SysAllocString and 6 reallocation.
This is an optimization that could be done in a similar way as strbuf_ensure_size() currently does: double the size of an allocated BSTR instead of allocating exactly old + new bytes for the final string. I can send an improved version of the patch if desired.