On 12/22/2012 12:12 AM, julien2412 wrote:
Cppcheck reported this:
[source/inprocserv/inprocembobj.cxx:783] ->
[source/inprocserv/inprocembobj.cxx:784]: (performance) Variable 'hr' is
reassigned a value before the old one has been used
Here are the lines:
778 HRESULT hr = m_pDefHandler->QueryInterface( IID_IOleObject,
(void**)&pOleObject );
779
780 ULONGGuard aGuard( &m_nCallsOnStack ); // avoid reentrance
problem
781 if ( SUCCEEDED( hr ) && pOleObject )
782 {
783 hr = pOleObject->Close( dwSaveOption );
784 hr = CoDisconnectObject(
(IUnknown*)(IPersistStorage*)this, 0 );
785 }
(see
http://opengrok.libreoffice.org/xref/core/embedserv/source/inprocserv/inprocembobj.cxx#778)
Moreover, the result of CoDisconnectObject isn't tested too.
I thought about adding after 783 and 784 (so after both lines) this:
if( !SUCCEEDED( hr ) ) return hr;
What do you think?
I really know zero about this, but I would /assume/ that (a) one wants
to indeed call CoDisconnectObject even if pOleObject->Close fails, and
(b) the return value of InprocEmbedDocument_Impl::Close should reflect
any failures returned from pOleObject->Close and CoDisconnectObject.
Something like
HRESULT ret = S_OK;
if ( m_pDefHandler ...
...
hr = pOleObject->Close(...);
if (!SUCCEEDED(hr))
ret = hr;
hr = CoDisconnectObject(...);
if (!(SUCCEEDED(hr) && SUCCEEDED(ret)))
ret = hr;
}
...
return ret;
Stephan
_______________________________________________
LibreOffice mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/libreoffice
