Re: [PATCH 5/6] d3drm: Implement IDirect3DRMTexture*::InitFromFile.

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Am 2016-05-08 um 11:31 schrieb Aaryaman Vasishta:
> +    /* d3drm intentionally leaks a reference to IDirect3DRM here if texture has already been initialized. */
> +    if (texture->d3drm)
> +        IDirect3DRM_AddRef(texture->d3drm);
> +
> +    if (texture->image)
> +        return D3DRMERR_BADOBJECT;
> +    if (FAILED(hr = d3drm_texture_load(texture, filename, FALSE)))
> +        return hr;
> Does it also leak if the file fails to load? It's probably better to create the image and then pass it to InitFromImage instead of re-coding this part.

Is there any situation where texture->d3drm would be NULL? The same question applies to the newly introduced check in d3drm_texture_destroy.
> +    /* Use an internal destroy callback to destroy image struct */
> +    hr = IDirect3DRMObject_AddDestroyCallback(&texture->IDirect3DRMTexture3_iface, destroy_image_callback, texture);
> +
> +cleanup:
> +    UnmapViewOfFile(buffer);
> +    CloseHandle(hmapping);
> +    CloseHandle(hfile);
> +
> +    return hr;
> You'll leak the image if AddDestroyCallback fails.

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJXL2hHAAoJEN0/YqbEcdMwezQP/0X9Ik1RbPNFRgvvFRxhrFQE
VLstrghPD28bQQ2i7YoVsdNCZMCyIQa+7GVzI/3d5pakyldEr/7OVIvHHGyxa8j/
YP2O0JTNGxVVhNB0pUM6bEPpnUj7BOPm0uCkcPhU42QVevzKBL7jUnl5Jx/j+j0K
Dld9+WDh3i2sSsrtFg2MLdsi4oG5ej3eOFuN2jibceK5hLR4UI1vwyyccCsD+Xd3
NmKnG6KMfiQsvVjxW7IoO5qDcmzZsKzdnMHFgL93KKkOkP2SBhODJckiWNNeswcr
XFjd6r9NBIZsFGdIBM040ph4gd/cu7h3R/IWw2hG++oQ+qxX0NOH61GNoUasjyQh
hKjyVqmjJa6MUPezzqoFv3XRPQGVnz4Q3l7GSp0QQYCJYtZR+8vey3ezkIuvfFNA
GszecmBvvi3Bv4Ir6JriJSUjTC73EcluKhS6WmFk+UxABrn0TBGSgFWkMENdxP6F
vE90h/a/JumNtdlCibpB/ctFP7NIWDs2Uz7RjrCQUPaXH86+hUtcs92farOi7JzV
EuwzXW1a2qgb7O6ZHWSxdCTvmJZy9rjS1gP8aH0gqUWaxr1FjJsPiOvSWU1/P1QX
qD/M/kENP4O0tmsI/dwnDRJ6lHf8FH71uNR0kRq2eWJa/clFdC+h5uMHRDx5w+iy
jlZ0/xkcdFzzaOJV1Yw0
=cwCD
-----END PGP SIGNATURE-----

[PATCH 5/6] d3drm: Implement IDirect3DRMTexture*::InitFromFile.

[Aaryaman Vasishta]

On Sun, May 8, 2016 at 9:54 PM, Stefan Dösinger <stefandoesinger(a)gmail.com>
wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> Am 2016-05-08 um 11:31 schrieb Aaryaman Vasishta:
> > +    /* d3drm intentionally leaks a reference to IDirect3DRM here if
> > texture has already been initialized. */
> > +    if (texture->d3drm)
> > +        IDirect3DRM_AddRef(texture->d3drm);
> > +
> > +    if (texture->image)
> > +        return D3DRMERR_BADOBJECT;
> > +    if (FAILED(hr = d3drm_texture_load(texture, filename, FALSE)))
> > +        return hr;
> > Does it also leak if the file fails to load? It's probably better to
> > create the image and then pass it to InitFromImage instead of re-coding
> > this part.
>
> Not afair while writing the tests, but it will leak if the texture was
> already initialized.
> Is there any situation where texture->d3drm would be NULL? The same
> question applies to the newly introduced check in d3drm_texture_destroy.

At this point, LoadTexture is not implemented yet. Currently LoadTexture
doesn't assign d3drm1, which means that check will be needed in case a
texture created by LoadTexture needs to be released. I've removed the NULL
check in the next patch after LoadTexture is implemented.

Cheers,
Aaryaman