<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 30, 2016 at 3:41 PM, Henri Verbeet <span dir="ltr"><<a href="mailto:hverbeet@gmail.com" target="_blank">hverbeet@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><br>
</span>You can probably fix the immediate issue just by storing the return<br>
value and releasing the reference if the QI failed.<br>
<br>
The larger issue is that code becomes much easier to follow when you<br>
can assume that functions are supposed to leave things (more or less)<br>
the way they found them on failure. (Unless explicitly indicated.)<br>
I.e., when d3drm_device_set_ddraw_device_d3d() releases all the<br>
references it created when it fails, and d3drm1_CreateDeviceFromD3D()<br>
can assume the device is in the same state as after calling<br>
d3drm_device_create().<br></blockquote><div>So I could e.g. release the reference, to ddraw, d3drm and d3d_device and set them all to NULL, effectively putting them in the same state as it it weas before calling d3drm_device_set_ddraw_device_d3d(). Is this fine?<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Of course the even larger issue is that<br>
d3drm_device_set_ddraw_device_d3d() doesn't make a lot of sense. In a<br>
way similar to e.g. d3drm3_CreateTexture(),<br>
d3drm1_CreateDeviceFromD3D() should probably just call<br>
IDirect3DRMDevice::InitFromD3D().<br></blockquote><div>Agreed. Back then it wasn't clear how CreateObject and Init worked together, though after the texture and viewport implementations, it became increasingly apparent that Init works only via CreateObject. A series of patches to move the implementations to Init could be done later on.<br></div><div>Thanks for the review!<br></div><div><br></div><div>Cheers,<br></div><div>Aaryaman <br></div></div><br></div></div>