Re: [PATCH] d3dx9_36/tests: Fix a memory leak in test_createtext.

[Matteo Bruni]

2017-02-27 7:52 GMT+01:00 Alex Henrie <alexhenrie24(a)gmail.com>:
> diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
> index 95dd06e466..64b7b8823c 100644
> --- a/dlls/d3dx9_36/tests/mesh.c
> +++ b/dlls/d3dx9_36/tests/mesh.c
> @@ -4340,8 +4340,7 @@ static void test_createtext(IDirect3DDevice9 *device, HDC hdc, const char *text,
>      if (!compute_text_mesh(&mesh, text, deviation, extrusion, otm.otmEMSquare, glyphs))
>      {
>          skip("Couldn't create mesh\n");
> -        d3dxmesh->lpVtbl->Release(d3dxmesh);
> -        return;
> +        goto error;
>      }
>      mesh.fvf = D3DFVF_XYZ | D3DFVF_NORMAL;

I would go in the other direction and strip all the error checking
branches and gotos from those tests. compute_text_mesh() returns FALSE
only on allocation failure and that's not supposed to happen in the
controlled conditions of the test. It's probably better to just fail
and crash in those cases.

[PATCH] d3dx9_36/tests: Fix a memory leak in test_createtext.

[Alex Henrie]

2017-02-28 8:21 GMT-07:00 Matteo Bruni <matteo.mystral(a)gmail.com>:
> 2017-02-27 7:52 GMT+01:00 Alex Henrie <alexhenrie24(a)gmail.com>:
> > diff --git a/dlls/d3dx9_36/tests/mesh.c b/dlls/d3dx9_36/tests/mesh.c
> > index 95dd06e466..64b7b8823c 100644
> > --- a/dlls/d3dx9_36/tests/mesh.c
> > +++ b/dlls/d3dx9_36/tests/mesh.c
> > @@ -4340,8 +4340,7 @@ static void test_createtext(IDirect3DDevice9 *device, HDC hdc, const char *text,
> >      if (!compute_text_mesh(&mesh, text, deviation, extrusion, otm.otmEMSquare, glyphs))
> >      {
> >          skip("Couldn't create mesh\n");
> > -        d3dxmesh->lpVtbl->Release(d3dxmesh);
> > -        return;
> > +        goto error;
> >      }
> >      mesh.fvf = D3DFVF_XYZ | D3DFVF_NORMAL;
>
> I would go in the other direction and strip all the error checking
> branches and gotos from those tests. compute_text_mesh() returns FALSE
> only on allocation failure and that's not supposed to happen in the
> controlled conditions of the test. It's probably better to just fail
> and crash in those cases.

Okay, I won't worry about it then.

-Alex