[PATCH v4 0/1] MR2132: vbscript: Fix memory leak in interp_redim_preserve

[Jason Millard (＠jsm174)]

While running in XCode's profiler, I noticed two memory leaks in `interp_redim_preserve`.

After looking at `interp_redim`, the `bounds` structure is freed.

I've updated `interp_redim_preserve` to free `bounds` when the array is NULL and not NULL.

--
  v4: vbscript: Fix memory leak in interp_redim_preserve

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

[PATCH v4 1/1] vbscript: Fix memory leak in interp_redim_preserve

[Jason Millard]

From: Jason Millard

---
 dlls/vbscript/interp.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c
index b3be3bea6cf..94c768c6e1e 100644
--- a/dlls/vbscript/interp.c
+++ b/dlls/vbscript/interp.c
@@ -1375,24 +1375,31 @@ static HRESULT interp_redim_preserve(exec_ctx_t *ctx)
     if(array == NULL || array->cDims == 0) {
         /* can initially allocate the array */
         array = SafeArrayCreate(VT_VARIANT, dim_cnt, bounds);
-        VariantClear(v);
-        V_VT(v) = VT_ARRAY|VT_VARIANT;
-        V_ARRAY(v) = array;
-        return S_OK;
+        if(!array)
+            hres = E_OUTOFMEMORY;
+        if(SUCCEEDED(hres)) {
+            VariantClear(v);
+            V_VT(v) = VT_ARRAY|VT_VARIANT;
+            V_ARRAY(v) = array;
+        }
     } else if(array->cDims != dim_cnt) {
         /* can't otherwise change the number of dimensions */
         TRACE("Can't resize %s, cDims %d != %d\n", debugstr_w(identifier), array->cDims, dim_cnt);
-        return MAKE_VBSERROR(VBSE_OUT_OF_BOUNDS);
+        hres = MAKE_VBSERROR(VBSE_OUT_OF_BOUNDS);
     } else {
         /* can resize the last dimensions (if others match */
         for(i = 0; i+1 < dim_cnt; ++i) {
             if(array->rgsabound[array->cDims - 1 - i].cElements != bounds[i].cElements) {
                 TRACE("Can't resize %s, bound[%d] %ld != %ld\n", debugstr_w(identifier), i, array->rgsabound[i].cElements, bounds[i].cElements);
-                return MAKE_VBSERROR(VBSE_OUT_OF_BOUNDS);
+                hres = MAKE_VBSERROR(VBSE_OUT_OF_BOUNDS);
+                break;
             }
         }
-        return SafeArrayRedim(array, &bounds[dim_cnt-1]);
+        if(SUCCEEDED(hres))
+            hres = SafeArrayRedim(array, &bounds[dim_cnt-1]);
     }
+    free(bounds);
+    return hres;
 }

static HRESULT interp_step(exec_ctx_t *ctx)

[PATCH v4 1/1] vbscript: Fix memory leak in interp_redim_preserve

[Dmitry Timoshkov]

Jason Millard  wrote:
> diff --git a/dlls/vbscript/interp.c b/dlls/vbscript/interp.c
> index b3be3bea6cf..94c768c6e1e 100644
> --- a/dlls/vbscript/interp.c
> +++ b/dlls/vbscript/interp.c
> @@ -1375,24 +1375,31 @@ static HRESULT interp_redim_preserve(exec_ctx_t *ctx)
>      if(array == NULL || array->cDims == 0) {
>          /* can initially allocate the array */
>          array = SafeArrayCreate(VT_VARIANT, dim_cnt, bounds);
> -
>    VariantClear(v);
> -
>    V_VT(v) = VT_ARRAY|VT_VARIANT;
> -
>    V_ARRAY(v) = array;
> -
>    return S_OK;
> -
>    if(!array)
> -
>        hres = E_OUTOFMEMORY;
> -
>    if(SUCCEEDED(hres)) {
> -
>        VariantClear(v);
> -
>        V_VT(v) = VT_ARRAY|VT_VARIANT;
> -
>        V_ARRAY(v) = array;
> -
>    }

It's not clear what kind of failure if(SUCCEEDED(hres)) is supposed to
check for. Probably this part would look more natural with
if (!array)
    hres = E_OUTOFMEMORY;
else {
    VariantClear(v);
    V_VT(v) = VT_ARRAY|VT_VARIANT;
    V_ARRAY(v) = array;
}

[Jason Millard (＠jsm174)]

On Wed Feb  8 14:44:21 2023 +0000, Jason Millard wrote:
> changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/2132/diffs?diff_id=31486&start_sha=2918bbad4099f9374078b928107c007b55f36951#19b309798e74fb8c8af366e0c6cc686d79a38532_1395_1373)

done

[PATCH v4 0/1] MR2132: vbscript: Fix memory leak in interp_redim_preserve - approved

[Jacek Caban (＠jacek)]

This merge request was approved by Jacek Caban.

[Jason Millard (＠jsm174)]

I can change this. I know this was already approved?