Re: [PATCH] ole32: OleGetClipboard(): zero output interface pointer on error
On 13.01.2016 13:23, Paul Gofman wrote:
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- dlls/ole32/clipboard.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/ole32/clipboard.c b/dlls/ole32/clipboard.c index ca9520f..b871bbc 100644 --- a/dlls/ole32/clipboard.c +++ b/dlls/ole32/clipboard.c @@ -2201,6 +2201,7 @@ HRESULT WINAPI OleGetClipboard(IDataObject **obj) TRACE("(%p)\n", obj);
if(!obj) return E_INVALIDARG; + *obj = NULL;
if(FAILED(hr = get_ole_clipbrd(&clipbrd))) return hr;
Hi, Paul. This is to handle a case when OLE was not initialized, right? If so, it should be easy to add a test for it. It makes sense to zero it as a common convention for out interface pointers, but a test won't hurt.
Hi Nikolay, I came through this while testing COM/OLE init stuff and tried to use OleGetClipboard to probe actual OLE state in Windows. The fact is that on Windows OleGetClipboard succeeds for me even if Ole is not initialized (and this is not the case in Wine). So straightforward test will be failing on Windows (returning S_OK when I expect and error in Wine). It is probably some separate compatibility issue. My test used to assign 0xdeadbeef to the interface pointer and release any non-null interface after the call regardless of status (as I saw as a common practice in ole tests), so it crashed under wine. I just thought it is appropriate to zero pointer on output as long it is a standard convention. I could possibly search for some conditions when OleGetClipboard fails on Windows to make a valid test like this. Does it make sense or better leave it as is for now? Thanks, Paul. On 01/13/2016 02:05 PM, Nikolay Sivov wrote:
On 13.01.2016 13:23, Paul Gofman wrote:
Signed-off-by: Paul Gofman <gofmanp(a)gmail.com> --- dlls/ole32/clipboard.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/dlls/ole32/clipboard.c b/dlls/ole32/clipboard.c index ca9520f..b871bbc 100644 --- a/dlls/ole32/clipboard.c +++ b/dlls/ole32/clipboard.c @@ -2201,6 +2201,7 @@ HRESULT WINAPI OleGetClipboard(IDataObject **obj) TRACE("(%p)\n", obj);
if(!obj) return E_INVALIDARG; + *obj = NULL;
if(FAILED(hr = get_ole_clipbrd(&clipbrd))) return hr;
Hi, Paul.
This is to handle a case when OLE was not initialized, right? If so, it should be easy to add a test for it. It makes sense to zero it as a common convention for out interface pointers, but a test won't hurt.
I managed to make Windows OleGetClipboard to fail by pre-locking the clipboard by OpenClipboard and verified that output pointer is zeroed. I should note that Wine OleGetClipboard does not fail in this case. Should I add such a test to this patch (with todo_wine on OleGetClipboard returning CLIPBRD_E_CANT_OPEN)? On 01/13/2016 02:18 PM, Paul Gofman wrote:
Hi Nikolay,
I came through this while testing COM/OLE init stuff and tried to use OleGetClipboard to probe actual OLE state in Windows. The fact is that on Windows OleGetClipboard succeeds for me even if Ole is not initialized (and this is not the case in Wine). So straightforward test will be failing on Windows (returning S_OK when I expect and error in Wine). It is probably some separate compatibility issue. My test used to assign 0xdeadbeef to the interface pointer and release any non-null interface after the call regardless of status (as I saw as a common practice in ole tests), so it crashed under wine. I just thought it is appropriate to zero pointer on output as long it is a standard convention. I could possibly search for some conditions when OleGetClipboard fails on Windows to make a valid test like this. Does it make sense or better leave it as is for now?
Thanks, Paul.
On 13.01.2016 15:00, Paul Gofman wrote:
I managed to make Windows OleGetClipboard to fail by pre-locking the clipboard by OpenClipboard and verified that output pointer is zeroed. I should note that Wine OleGetClipboard does not fail in this case. Should I add such a test to this patch (with todo_wine on OleGetClipboard returning CLIPBRD_E_CANT_OPEN)?
Sure, if tests are reliable on Windows and we don't have them already it won't hurt to add them.
On 01/13/2016 02:18 PM, Paul Gofman wrote:
Hi Nikolay,
I came through this while testing COM/OLE init stuff and tried to use OleGetClipboard to probe actual OLE state in Windows. The fact is that on Windows OleGetClipboard succeeds for me even if Ole is not initialized (and this is not the case in Wine). So straightforward test will be failing on Windows (returning S_OK when I expect and error in Wine). It is probably some separate compatibility issue. My test used to assign 0xdeadbeef to the interface pointer and release any non-null interface after the call regardless of status (as I saw as a common practice in ole tests), so it crashed under wine. I just thought it is appropriate to zero pointer on output as long it is a standard convention. I could possibly search for some conditions when OleGetClipboard fails on Windows to make a valid test like this. Does it make sense or better leave it as is for now?
So when OLE is not initialized on Windows it returns S_OK and valid interface pointer? It's better to test this with minimal application, not wine tests, to avoid any possible side effects of previous init/uninit sequences. Or just make it a very first test before first attempt to initialize.
Thanks, Paul.
On 01/13/2016 09:20 PM, Nikolay Sivov wrote:
On 13.01.2016 15:00, Paul Gofman wrote:
I managed to make Windows OleGetClipboard to fail by pre-locking the clipboard by OpenClipboard and verified that output pointer is zeroed. I should note that Wine OleGetClipboard does not fail in this case. Should I add such a test to this patch (with todo_wine on OleGetClipboard returning CLIPBRD_E_CANT_OPEN)? Sure, if tests are reliable on Windows and we don't have them already it won't hurt to add them. I will add such test to this patch which will also verify that output pointer is natively zeroed on error (it is not a complete verification of course as in theory Windows may behave differently on different errors, though I suppose this is very unlikely). On 01/13/2016 02:18 PM, Paul Gofman wrote:
So when OLE is not initialized on Windows it returns S_OK and valid interface pointer? It's better to test this with minimal application, not wine tests, to avoid any possible side effects of previous init/uninit sequences. Or just make it a very first test before first attempt to initialize.
Yes, this is the case (at least in my WinXP virtual machine). The same is when I make this test first (before any initializations, right at the beginning of top-level test function). I can add such a test but presumably as a separate patch as it is not related to zeroing pointer on error (I could not get OleGetClipboard failing on Windows due to unitialized OLE so far).
participants (2)
-
Nikolay Sivov -
Paul Gofman