On Thu, 7 Mar 2019 at 22:42, Stefan Dösinger stefan@codeweavers.com wrote:
Am 05.03.2019 um 23:16 schrieb Henri Verbeet hverbeet@gmail.com: That's pretty ugly.
I know. But which part do you think could be improved in particular? Moving the extra checks out of impl_from_IDirectDrawClipper to keep it in line with the standard way we do COM objects?
Mostly the fact that impl_from_IDirectDrawClipper() can return NULL, yes. I'm not entirely sure ddraw needs more magic either, but I could live with that part if needed.
Would it work to introduce a helper along the lines of the floowing?
static BOOL ddraw_clipper_is_valid(struct ddraw_clipper *clipper) { return !IsBadReadPtr(clipper, sizeof(*clipper)) && clipper->IDirectDrawClipper_iface.lpVtbl == &ddraw_clipper_vtbl; }
The vtable only matters for Release(), the rest of the methods don't care if you change the vtable, that's why I added the magic value.
Do applications care though? In principle impl_from_IDirectDrawClipper() doesn't currently check the vtable, but in practice passing a clipper with a modified vtable to just about anything is going to trigger the assert in unsafe_impl_from_IDirectDrawClipper(). I.e., as long as applications aren't actually modifying the vtable and still expect things to work, I'd probably prefer using the vtable as "magic".