[Libreoffice] [PATCH] possible null-dereferencing found by cppcheck

[Takeshi Abe]

On Tue, 18 Jan 2011 03:26:14 -0700, "Tor Lillqvist" <tlillqvist at novell.com> wrote:
> Ah OK. I see that now when I pulled a fresher version.
> 
> But unless I am mistaken, now then pArgs might in theory be de-references while NULL?
> 
> Consider this code path:
> 
>     const SfxItemSet* pArgs = rReq.GetArgs();
> 
>     SFX_REQUEST_ARG (rReq, pHelpLineIndex, SfxUInt32Item, ID_VAL_INDEX, FALSE);
>     // Assume pHelpLineIndex gets set to non-NULL
>    if (pHelpLineIndex != NULL)
>     {
>         // so pArgs gets set to NULL
>         pArgs = NULL;
>     }
>     
>     if ( !pArgs )
>     {
>         // Thus this block is entered
> 
>         SdAbstractDialogFactory* pFact = SdAbstractDialogFactory::Create();
>         // Assume pFact gets set to NULL. Clearly that is possible as the code right after bothers to check for it?
>        AbstractSdSnapLineDlg* pDlg = pFact ? pFact->CreateSdSnapLineDlg( NULL, aNewAttr, mpView ) : 0;
>         // and thus pDlg is NULL
>         if( pDlg )
>         {
>             // so this block is not entered, which is the only place where pArgs gets 
>             // set to non-NULL.
>         }
>     }
>     // Thus pArgs can be NULL here
>     aHlpPos.X() = ((const SfxUInt32Item&) pArgs->Get(ATTR_SNAPLINE_X)).GetValue();
>     aHlpPos.Y() = ((const SfxUInt32Item&) pArgs->Get(ATTR_SNAPLINE_Y)).GetValue();
> 
> Or am I missing something... 
Yes, we have a miserable trap of fall-through:

            switch( nResult )
            {
                case RET_OK:
                    rReq.Done(aNewAttr);
                    pArgs = rReq.GetArgs();
                    break;

                case RET_SNAP_DELETE:
                    // Fangobjekt loeschen
                    if ( !bCreateNew )
                        pPV->DeleteHelpLine(nHelpLine);
                    // und weiter wie bei default
                    /*fall-through*/
                default:
                    return;
            }

Please apply the attached patch saving us in future.

Cheers,
-- Takeshi Abe