[PATCH v3 0/1] MR10396: Draft: vbscript: Implement IActiveScriptError::GetSourceLineText.

[Francis De Brabandere (＠francisdb)]

Extract the error source line in report_script_error() and store it
in VBScriptError so that GetSourceLineText can return it. Also fix
a memory leak in VBScriptError_Release where EXCEPINFO BSTRs were
not freed.

--
  v3: vbscript: Implement IActiveScriptError::GetSourceLineText.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10396

[PATCH v3 1/1] vbscript: Implement IActiveScriptError::GetSourceLineText.

[Francis De Brabandere]

From: Francis De Brabandere <[e-mail]>

Extract the error source line in report_script_error() and store it
in VBScriptError so that GetSourceLineText can return it. Also fix
a memory leak in VBScriptError_Release where EXCEPINFO BSTRs were
not freed.
---
 dlls/vbscript/tests/run.c | 72 ++++++++++++++++++++++++++++-----------
 dlls/vbscript/vbscript.c  | 30 +++++++++++++---
 2 files changed, 78 insertions(+), 24 deletions(-)

diff --git a/dlls/vbscript/tests/run.c b/dlls/vbscript/tests/run.c
index 4b69bc18ffa..12eea054fe9 100644
--- a/dlls/vbscript/tests/run.c
+++ b/dlls/vbscript/tests/run.c
@@ -2007,6 +2007,7 @@ static HRESULT WINAPI ActiveScriptSite_OnStateChange(IActiveScriptSite *iface, S
 static IActiveScriptError **store_script_error;
 static ULONG error_line;
 static LONG error_char;
+static BSTR error_source_line;
 
 static HRESULT WINAPI ActiveScriptSite_OnScriptError(IActiveScriptSite *iface, IActiveScriptError *pscripterror)
 {
@@ -2015,6 +2016,10 @@ static HRESULT WINAPI ActiveScriptSite_OnScriptError(IActiveScriptSite *iface, I
     hres = IActiveScriptError_GetSourcePosition(pscripterror, NULL, &error_line, &error_char);
     ok(hres == S_OK, "GetSourcePosition failed: %08lx\n", hres);
 
+    SysFreeString(error_source_line);
+    error_source_line = NULL;
+    IActiveScriptError_GetSourceLineText(pscripterror, &error_source_line);
+
     if(!expect_OnScriptError) {
         EXCEPINFO info;
 
@@ -2696,6 +2701,7 @@ static void test_parse_errors(void)
         const WCHAR *src;
         unsigned error_line;
         int error_char;
+        const WCHAR *source_line;
     }
     invalid_scripts[] =
     {
@@ -2703,7 +2709,8 @@ static void test_parse_errors(void)
             /* If...End If */
             L"If 0 > 1 Then\n"
             "    x = 0 End If\n",
-            1, 10
+            1, 10,
+            L"    x = 0 End If"
         },
         {
             /* ElseIf...End If */
@@ -2711,50 +2718,58 @@ static void test_parse_errors(void)
             "    x = 0\n"
             "ElseIf True Then\n"
             "    x = 1 End If\n",
-            3, 10
+            3, 10,
+            L"    x = 1 End If"
         },
         {
             /* Else End If (no separator) */
             L"If False Then\n"
             "    x = 0\n"
             "Else End If\n",
-            2, 5
+            2, 5,
+            L"Else End If"
         },
         {
             /* While...End While */
             L"While False\n"
             "    x = 0 End While\n",
-            1, 10
+            1, 10,
+            L"    x = 0 End While"
         },
         {
             /* While...Wend */
             L"While False\n"
             "    x = 0 Wend\n",
-            1, 10
+            1, 10,
+            L"    x = 0 Wend"
         },
         {
             /* Do While...Loop */
             L"Do While False\n"
             "    x = 0 Loop\n",
-            1, 10
+            1, 10,
+            L"    x = 0 Loop"
         },
         {
             /* Do Until...Loop */
             L"Do Until True\n"
             "    x = 0 Loop\n",
-            1, 10
+            1, 10,
+            L"    x = 0 Loop"
         },
         {
             /* Do...Loop While */
             L"Do\n"
             "    x = 0 Loop While False\n",
-            1, 10
+            1, 10,
+            L"    x = 0 Loop While False"
         },
         {
             /* Do...Loop Until */
             L"Do\n"
             "    x = 0 Loop Until True\n",
-            1, 10
+            1, 10,
+            L"    x = 0 Loop Until True"
         },
         {
             /* Select...End Select */
@@ -2765,36 +2780,43 @@ static void test_parse_errors(void)
             "    Case 42\n"
             "        x = True End Select\n"
             "Call ok(x, \"wrong case\")\n",
-            5, 17
+            5, 17,
+            L"        x = True End Select"
         },
         {
             /* Class...End Class  (empty) */
             L"Class C End Class",
-            0, 8
+            0, 8,
+            L"Class C End Class"
         },
         {
             /* Class...End Class  (empty) */
             L"Class C _\nEnd Class",
-            1, 0
+            1, 0,
+            L"End Class"
         },
         {
             /* invalid use of parentheses for call statement */
             L"strcomp(\"x\", \"y\")",
-            0, -17
+            0, -17,
+            L"strcomp(\"x\", \"y\")"
         },
         {
             L"\n\n\n  cint _\n   throwInt(&h80001234&)",
-            3, 2
+            3, 2,
+            NULL
         },
         {
             L"dim x\n"
             "if true then throwInt(&h80001234&)",
-            1, 13
+            1, 13,
+            NULL
         },
         {
             L"dim x\n"
             "if x = throwInt(&h80001234&) then x = 1",
-            1, 0
+            1, 0,
+            NULL
         },
         {
             L"sub test\n"
@@ -2802,14 +2824,16 @@ static void test_parse_errors(void)
             "    if x = throwInt(&h80001234&) then x = 1\n"
             "end sub\n"
             "test\n",
-            2, 4
+            2, 4,
+            NULL
         },
         {
             L"dim x\n"
             "do\n"
             "    x = 1\n"
             "loop until throwInt(&h80001234&)\n",
-            3, 0
+            3, 0,
+            NULL
         },
         {
             L"\n  select case 3\n"
@@ -2818,7 +2842,8 @@ static void test_parse_errors(void)
             "    case throwInt(&h80001234&)\n"
             "        throwInt &h87001234&\n"
             "end select\n",
-            1, 2
+            1, 2,
+            NULL
         },
         {
             L"if false then\n"
@@ -2828,7 +2853,8 @@ static void test_parse_errors(void)
             "else\n"
             "    throwInt &h87001234&\n"
             "end if\n",
-            2, 1
+            2, 1,
+            NULL
         }
     };
     HRESULT hres;
@@ -2850,7 +2876,13 @@ static void test_parse_errors(void)
         todo_wine_if(invalid_scripts[i].error_char < 0)
         ok(error_char == abs(invalid_scripts[i].error_char), "[%u] error char %ld expected %d\n",
            i, error_char, invalid_scripts[i].error_char);
+        if(invalid_scripts[i].source_line)
+            ok(!wcscmp(error_source_line, invalid_scripts[i].source_line),
+               "[%u] source line %s expected %s\n", i,
+               wine_dbgstr_w(error_source_line), wine_dbgstr_w(invalid_scripts[i].source_line));
     }
+    SysFreeString(error_source_line);
+    error_source_line = NULL;
 }
 
 static void test_msgbox(void)
diff --git a/dlls/vbscript/vbscript.c b/dlls/vbscript/vbscript.c
index 715c98d32bb..2a60dff5735 100644
--- a/dlls/vbscript/vbscript.c
+++ b/dlls/vbscript/vbscript.c
@@ -64,6 +64,7 @@ typedef struct {
     DWORD_PTR cookie;
     unsigned line;
     unsigned character;
+    BSTR source_line;
 } VBScriptError;
 
 static inline WCHAR *heap_pool_strdup(heap_pool_t *heap, const WCHAR *str)
@@ -505,8 +506,13 @@ static ULONG WINAPI VBScriptError_Release(IActiveScriptError *iface)
 
     TRACE("(%p) ref=%ld\n", This, ref);
 
-    if(!ref)
+    if(!ref) {
+        SysFreeString(This->source_line);
+        SysFreeString(This->ei.bstrSource);
+        SysFreeString(This->ei.bstrDescription);
+        SysFreeString(This->ei.bstrHelpFile);
         free(This);
+    }
 
     return ref;
 }
@@ -542,8 +548,19 @@ static HRESULT WINAPI VBScriptError_GetSourcePosition(IActiveScriptError *iface,
 static HRESULT WINAPI VBScriptError_GetSourceLineText(IActiveScriptError *iface, BSTR *source)
 {
     VBScriptError *This = impl_from_IActiveScriptError(iface);
-    FIXME("(%p)->(%p)\n", This, source);
-    return E_NOTIMPL;
+
+    TRACE("(%p)->(%p)\n", This, source);
+
+    if(!source)
+        return E_POINTER;
+
+    if(!This->source_line) {
+        *source = NULL;
+        return E_FAIL;
+    }
+
+    *source = SysAllocString(This->source_line);
+    return *source ? S_OK : E_OUTOFMEMORY;
 }
 
 static const IActiveScriptErrorVtbl VBScriptErrorVtbl = {
@@ -558,7 +575,7 @@ static const IActiveScriptErrorVtbl VBScriptErrorVtbl = {
 HRESULT report_script_error(script_ctx_t *ctx, const vbscode_t *code, unsigned loc)
 {
     VBScriptError *error;
-    const WCHAR *p, *nl;
+    const WCHAR *p, *nl, *line_end;
     HRESULT hres, result;
 
     if(!(error = malloc(sizeof(*error))))
@@ -580,6 +597,11 @@ HRESULT report_script_error(script_ctx_t *ctx, const vbscode_t *code, unsigned l
     }
     error->character = code->source + loc - nl;
 
+    line_end = nl;
+    while(*line_end && *line_end != '\n' && *line_end != '\r')
+        line_end++;
+    error->source_line = SysAllocStringLen(nl, line_end - nl);
+
     hres = IActiveScriptSite_OnScriptError(ctx->site, &error->IActiveScriptError_iface);
     IActiveScriptError_Release(&error->IActiveScriptError_iface);
     return hres == S_OK ? SCRIPT_E_REPORTED : result;
-- 
GitLab

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

[PATCH v3 0/1] MR10396: vbscript: Implement IActiveScriptError::GetSourceLineText.

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/vbscript/vbscript.c:
> }
>      error->character = code->source + loc - nl;
>
> +    line_end = nl;
> +    while(*line_end && *line_end != '\n' && *line_end != '\r')
> +        line_end++;
> +    error->source_line = SysAllocStringLen(nl, line_end - nl);

Maybe instead of storing another string in the object, which requires allocation, we could just store a reference to `vbcode_t` and get the line from there on demand.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10396#note_133023

[PATCH v3 0/1] MR10396: vbscript: Implement IActiveScriptError::GetSourceLineText.

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/vbscript/vbscript.c:
> TRACE("(%p) ref=%ld\n", This, ref);
>
> -    if(!ref)
> +    if(!ref) {
> +        SysFreeString(This->source_line);
> +        SysFreeString(This->ei.bstrSource);
> +        SysFreeString(This->ei.bstrDescription);
> +        SysFreeString(This->ei.bstrHelpFile);

This could use `clear_ei`.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10396#note_133024

[PATCH v3 0/1] MR10396: vbscript: Implement IActiveScriptError::GetSourceLineText.

[Jacek Caban (＠jacek)]

Jacek Caban (@jacek) commented about dlls/vbscript/tests/run.c:
> todo_wine_if(invalid_scripts[i].error_char < 0)
>          ok(error_char == abs(invalid_scripts[i].error_char), "[%u] error char %ld expected %d\n",
>             i, error_char, invalid_scripts[i].error_char);
> +        if(invalid_scripts[i].source_line)

Could we test failure case too?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10396#note_133025