Re: Cppcheck: reassignment of m_bInChangingState (dbaccess module)

On Wed, Jun 12, 2013 at 01:08:48PM -0700, julien2412 wrote:

> Cppcheck reported this:
> [source/core/dataaccess/documentdefinition.cxx:211] ->
> [source/core/dataaccess/documentdefinition.cxx:212]: (performance,
> inconclusive) Variable 'm_bInChangingState' is reassigned a value before the
> old one has been used if variable is no semaphore variable
> Indeed:
>     207     void SAL_CALL OEmbedObjectHolder::changingState( const
> lang::EventObject& /*aEvent*/, ::sal_Int32 nOldState, ::sal_Int32 nNewState
> ) throw (embed::WrongStateException, uno::RuntimeException)
>     208     {
>     209         if ( !m_bInChangingState && nNewState ==
> EmbedStates::RUNNING && nOldState == EmbedStates::ACTIVE && m_pDefinition )
>     210         {
>     211             m_bInChangingState = true;
>     212             m_bInChangingState = false;
>     213         }
>     214     }

> Is there something lacking between both (like
> OEmbedObjectHolder::stateChanged function, just some lines below)?

Well, this code appeared in:

commit 66de7dc15ba8e6f04eccd755fcc7494e98ffd46d
Author: Rüdiger Timm <rt@openoffice.org>
Date:   Wed Jan 30 07:34:38 2008 +0000

    INTEGRATION: CWS dba24d (1.48.6); FILE MERGED

as:

        if ( !m_bInChangingState && nNewState == EmbedStates::RUNNING && nOldState == 
EmbedStates::ACTIVE && m_pDe
        {
            m_bInChangingState = true;
            //m_pDefinition->save(sal_False);
            m_bInChangingState = false;
        }

Which suggests it could be buggy in the way that "m_pDefinition->save"
should *not* be commented out. Or maybe not. Dunno. Have to look at
what OEmbedObjectHolder::changingState does, when it is called, etc.

The commented out "m_pDefinition->save" was later removed, but as part
of a general "dumb" cleanup of "remove all commented-out code", so I
don't trust it.

It seems to be the implementation of the interface
com/sun/star/embed/XStateChangeListener.idl

The OEmbedObjectHolder class is local to that file. It is created and
assigned to m_xListener member of ODocumentDefinition. But I don't see
any *use* of that m_xListener at all, and it is a private
member. <sigh> It looks like someone, way back in 2008
half-implemented something but never finished it...

-- 
Lionel