Re: [PATCH] ole32: check for interface NULL which happens with e.g. Abiword
Marcus Meissner <marcus(a)jet.franken.de> wrote:
+ if (!unk) { + FIXME("hr was %d, but unk is NULL?\n", hr); + return E_FAIL; + }
IDropTarget_QueryInterface() should be fixed instead. -- Dmitry.
On Wed, Sep 15, 2010 at 08:04:51PM +0900, Dmitry Timoshkov wrote:
Marcus Meissner <marcus(a)jet.franken.de> wrote:
+ if (!unk) { + FIXME("hr was %d, but unk is NULL?\n", hr); + return E_FAIL; + }
IDropTarget_QueryInterface() should be fixed instead.
The implementation is in abiword, so that would be tricky. Marcus, you're leaking a stream in this error path. Better to do something like: if(FAILED(hr) || !unk) Huw.
On Wed, Sep 15, 2010 at 12:22:57PM +0100, Huw Davies wrote:
On Wed, Sep 15, 2010 at 08:04:51PM +0900, Dmitry Timoshkov wrote:
Marcus Meissner <marcus(a)jet.franken.de> wrote:
+ if (!unk) { + FIXME("hr was %d, but unk is NULL?\n", hr); + return E_FAIL; + }
IDropTarget_QueryInterface() should be fixed instead.
The implementation is in abiword, so that would be tricky.
Marcus, you're leaking a stream in this error path. Better to do something like: if(FAILED(hr) || !unk)
yeah, but what error to return :/ Ciao, Marcus
On Wed, Sep 15, 2010 at 01:34:06PM +0200, Marcus Meissner wrote:
On Wed, Sep 15, 2010 at 12:22:57PM +0100, Huw Davies wrote:
On Wed, Sep 15, 2010 at 08:04:51PM +0900, Dmitry Timoshkov wrote:
Marcus Meissner <marcus(a)jet.franken.de> wrote:
+ if (!unk) { + FIXME("hr was %d, but unk is NULL?\n", hr); + return E_FAIL; + }
IDropTarget_QueryInterface() should be fixed instead.
The implementation is in abiword, so that would be tricky.
Marcus, you're leaking a stream in this error path. Better to do something like: if(FAILED(hr) || !unk)
yeah, but what error to return :/
Yeah, sorry. Something like: if(!unk) hr = E_NOINTERFACE; right after the QI call. Huw.
On Wed, Sep 15, 2010 at 12:40:46PM +0100, Huw Davies wrote:
On Wed, Sep 15, 2010 at 01:34:06PM +0200, Marcus Meissner wrote:
On Wed, Sep 15, 2010 at 12:22:57PM +0100, Huw Davies wrote:
On Wed, Sep 15, 2010 at 08:04:51PM +0900, Dmitry Timoshkov wrote:
Marcus Meissner <marcus(a)jet.franken.de> wrote:
+ if (!unk) { + FIXME("hr was %d, but unk is NULL?\n", hr); + return E_FAIL; + }
IDropTarget_QueryInterface() should be fixed instead.
The implementation is in abiword, so that would be tricky.
Marcus, you're leaking a stream in this error path. Better to do something like: if(FAILED(hr) || !unk)
yeah, but what error to return :/
Yeah, sorry. Something like: if(!unk) hr = E_NOINTERFACE; right after the QI call.
I ran testbot, https://testbot.winehq.org/JobDetails.pl?Key=5239 and it also has 0 as hr for the case. So returning failure might not be right approach. Abiword has this code btw: STDMETHODIMP XAP_Win32DropTarget::QueryInterface(REFIID /*riid*/, LPVOID FAR* /*ppvObj*/) { return S_OK; } The fun... Would erroring out be right? If an interface directly inherits IUnknown, do we need a QueryInterface or could we just cast? Ciao, Marcus
On Wed, Sep 15, 2010 at 08:04:51PM +0900, Dmitry Timoshkov wrote:
Marcus Meissner <marcus(a)jet.franken.de> wrote:
+ if (!unk) { + FIXME("hr was %d, but unk is NULL?\n", hr); + return E_FAIL; + }
IDropTarget_QueryInterface() should be fixed instead.
I am not sure where where the DropTarget object comes from. If it comes out of the marshaler, then its too much magic ;) IRC identified in the meantime: commit 6d1ef3a6a64f0fabf05ce1bba5f0ec4373684786 13:15 < igorko> Author: Huw Davies <huw(a)codeweavers.com> 13:15 < igorko> Date: Thu Jul 22 13:37:19 2010 +0100 13:15 < igorko> ole32: Implement cross-process drag and drop. 13:16 < igorko> :040000 040000 ab88cc6bf0936bc4adc7bff0673415282613d919 13:16 < igorko> 065c727204a46d01708bc01f10484cad8527e1a2 M dlls Ciao, Marcus
participants (3)
-
Dmitry Timoshkov -
Huw Davies -
Marcus Meissner