On 26.04.2016 16:33, Andrew Eikum wrote:
On Mon, Apr 25, 2016 at 11:46:09PM -0600, Alex Henrie wrote:
2016-04-25 23:10 GMT-06:00 Nikolay Sivov bunglehead@gmail.com:
On 26.04.2016 7:57, Alex Henrie wrote:
2016-04-25 22:21 GMT-06:00 Nikolay Sivov bunglehead@gmail.com:
On 26.04.2016 6:10, Alex Henrie wrote:
static HRESULT WINAPI TimelineObj_GetTimelineNoRef(IAMTimelineObj *iface, IAMTimeline **timeline) {
- /* MSDN says that this function is "not supported" */ TimelineObjImpl *This = impl_from_IAMTimelineObj(iface);
- FIXME("(%p)->(%p): not implemented!\n", This, timeline);
- return E_NOTIMPL;
- TRACE("(%p)->(%p)\n", This, timeline);
- if (!timeline) return E_POINTER;
- return E_NOINTERFACE;
}
This looks like a call to QI, according to your tests.
It looks that way, but which interface would it be querying? The tests show that an IAMTimelineObj cannot double as an IAMTimeline, nor is the function supposed to return the IAMTimeline that was used to create the IAMTimelineObj. My best guess is that Microsoft never bothered implementing this function properly and so whatever implementation they have always winds up returning E_NOINTERFACE.
Yes, return type is IAMTimeline, so I was suggesting a shortcut doing same thing in one line IAMTimelineObj_QueryInterface(iface, &IID_IAMTimeline, ...). This should cover for setting out pointer to NULL, returning E_NOINTERFACE, and probably handling NULL case too, though it's not a standard QI behavior. None of that is actually important, what you did is fine, take it as a suggestion, or don't.
OK, I see what you mean now, and I'll make that change in version 2 of this patchset.
Eh, I'd rather keep it like it was. It's not clear whether it should be QIing on this same object or on the parent Timeline object, and the "NoRef" is weird behavior for a QI. Implementing it on top of QI seems to imply confidence we don't have, and would confuse me if I was reading this code blind.
I agree, it's better to keep it simple, and NoRef implies something non-standard. Sorry for the noise.
Andrew