About coverity 738980 (ssfrm.cxx from sw module)

[Caolán McNamara]

On Fri, 2013-03-15 at 16:31 -0700, julien2412 wrote:
> Hello,
> 
> Scan coverity detected this: 
> 22. pass_freed_arg: Passing freed pointer "pAnchoredObj" as an argument to
> function "SwSortedObjs::Remove(SwAnchoredObject &)".
> 
> This problem appears twice in the file:
>     576                 SwAnchoredObject* pAnchoredObj =
> (*pFrm->GetDrawObjs())[0];
>     577                 if ( pAnchoredObj->ISA(SwFlyFrm) )
>     578                     delete pAnchoredObj;
>     579                 else
>     580                 {
>     581                     SdrObject* pSdrObj = pAnchoredObj->DrawObj();
>     582                     SwDrawContact* pContact =
>     583                            
> static_cast<SwDrawContact*>(pSdrObj->GetUserCall());
>     584                     OSL_ENSURE( pContact,
>     585                             "<SwFrm::~SwFrm> - missing contact for
> drawing object" );
>     586                     if ( pContact )
>     587                     {
>     588                         pContact->DisconnectObjFromLayout( pSdrObj
> );
>     589                     }
>     590                 }
>     591                 if ( pFrm->GetDrawObjs() &&
>     592                      nCnt == pFrm->GetDrawObjs()->Count() )
>     593                 {
>     594                     pFrm->GetDrawObjs()->Remove( *pAnchoredObj ); 
> //// pAnchoredObj has been deleted!
>     595                 }

> Should "delete pAnchoredObj;" be followed by pAnchoredObj = null; in the if
> part then  if ( GetDrawObjs() && nCnt == GetDrawObjs()->Count() ) should be
> replaced by:
>  if ( GetDrawObjs() && nCnt == GetDrawObjs()->Count()  && pAnchoredObj) ?
> 
> Or:
>  if ( GetDrawObjs() && nCnt == GetDrawObjs()->Count() ) part included in
> else block?

I think coverity is excited here because its dereferencing a deleted
pointer. What the SwSortedObjs does is take the address of the
referenced object, i.e. gets the original pointer value again, and
removes that from its list of "alive objects".

So pAnchoredObj = NULL and/or moving it into the else block might be
both wrong. What might make coverity happier is to change SwSortedObjs
so that its "Insert" and "Remove" take a pointer and not reference.

C.