On 29 June 2016 at 17:54, Aaryaman Vasishta jem456.vasishta@gmail.com wrote:
On Wed, Jun 29, 2016 at 8:01 PM, Henri Verbeet hverbeet@gmail.com wrote:
On 29 June 2016 at 16:13, Aaryaman Vasishta jem456.vasishta@gmail.com wrote:
This is a helper function, so the caller would be handling the references. See CreateDeviceFromD3D in device.c.
That construction makes it harder to verify correctness, so in general I'd recommend against that. Regardless, d3drm_device_destroy() takes care of the device reference, but doesn't help for the IDirect3DRM reference, since that's only released when device->ddraw is set.
device->ddraw is set while returning. What should be the better approach here?
You can probably fix the immediate issue just by storing the return value and releasing the reference if the QI failed.
The larger issue is that code becomes much easier to follow when you can assume that functions are supposed to leave things (more or less) the way they found them on failure. (Unless explicitly indicated.) I.e., when d3drm_device_set_ddraw_device_d3d() releases all the references it created when it fails, and d3drm1_CreateDeviceFromD3D() can assume the device is in the same state as after calling d3drm_device_create().
Of course the even larger issue is that d3drm_device_set_ddraw_device_d3d() doesn't make a lot of sense. In a way similar to e.g. d3drm3_CreateTexture(), d3drm1_CreateDeviceFromD3D() should probably just call IDirect3DRMDevice::InitFromD3D().