<div dir="ltr"><div>I've fixed whatever leaks I could find, and also attached implementations of CreateDeviceFromSurface. Do take a look and if possible, provide feedback on any leaks that I might have missed.<br><br><br></div>Jam<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Jul 26, 2015 at 5:34 PM, Aaryaman Vasishta <span dir="ltr"><<a href="mailto:jem456.vasishta@gmail.com" target="_blank">jem456.vasishta@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Sun, Jul 26, 2015 at 4:33 PM, Stefan Dösinger <span dir="ltr"><<a href="mailto:stefandoesinger@gmail.com" target="_blank">stefandoesinger@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>-----BEGIN PGP SIGNED MESSAGE-----<br>
Hash: SHA1<br>
<br>
</span>Hi,<br>
<br>
I like the overall structure a lot more than the previous incarnation!<br>
<br>
Here are a few more comments:<br>
<br>
> + hr = DirectDrawCreate(NULL, &ddraw, NULL);<br>
<span>> + if (FAILED(hr))<br>
> + return hr;<br>
> +<br>
</span>> hr = Direct3DRMDevice_create(&object);<br>
> if (FAILED(hr))<br>
> return hr;<br>
You're leaking "ddraw" here. There are many more incomplete error handling paths in the patches.<br></blockquote></span><div>I'll post another incarnation with all the leaks I can possibly find fixed :) <br></div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +HRESULT set_clipper_surface(struct d3drm_device *object, IDirectDraw *ddraw, IDirectDrawClipper *clipper, int width, int height,<br>
> + IDirectDrawSurface **surface) DECLSPEC_HIDDEN;<br>
I'm not entirely happy with this name, because it doesn't merely set any surfaces, it creates them. What do you think about d3drm_device_create_surfaces_from_clipper? Check for consistency wrt the "d3drm_device" part. E.g. the next function you could name "d3drm_device_init" or just "device_init".<br></blockquote></span><div>It took me a while to figure out a name, since a name like d3drm_device_create_surfaces_from_clipper felt too long for me initially. But if long function names are accepted then I don't mind putting that name up. </div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> +HRESULT init_device(struct d3drm_device *device, UINT version, IDirect3DRM *d3drm, IDirectDraw *ddraw, IDirectDrawSurface *surface, BOOL z_surface, IDirect3D2 *d3d2,<br>
> + int width, int height)<br>
"BOOL z_surface" is unused right now. It won't be needed until you add CreateDeviceFromSurface version 3.<br></blockquote></span><div>Right, I'll remove this for now and re-add it in the CreateDeviceFromSurface patches. Note that z_surface is still needed in version 1 and 2 of CreateDeviceFromSurface in the cases where the surface already has ds attached to it by the application (in which case native wouldn't create its own ds internally, as seen by the tests). <br></div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span></span><span><br>
</span>You are leaking ds here in the success case. You can release it after calling AddAttachedSurface.<br></blockquote></span><div>I'm not sure why I allowed that leak to happen, but I believe it was causing some refcounting/segfault issues if I released the ds there. I'll get back with more details about this. <br><br><br><br></div><div>Jam<br></div></div></div></div>
</blockquote></div><br></div>