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

Hello,

So, in that patch, remove also the German comment that says something
similar in different words.

Cheers

F.

On Tue, 2011-01-18 at 23:27 +0900, Takeshi Abe wrote: 
> On Tue, 18 Jan 2011 03:26:14 -0700, "Tor Lillqvist" <tlillqvist@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
> differences between files attachment (0001-mark-fall-through.patch)
> From e560833d8151a0c5745723e0c528c389d5618849 Mon Sep 17 00:00:00 2001
> From: Takeshi Abe <tabe@fixedpoint.jp>
> Date: Tue, 18 Jan 2011 23:22:17 +0900
> Subject: [PATCH] mark fall-through
>
> ---
>  sd/source/ui/func/fusnapln.cxx |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/sd/source/ui/func/fusnapln.cxx b/sd/source/ui/func/fusnapln.cxx
> index d544ec3..2bc2b01 100644
> --- a/sd/source/ui/func/fusnapln.cxx
> +++ b/sd/source/ui/func/fusnapln.cxx
> @@ -174,6 +174,7 @@ void FuSnapLine::DoExecute( SfxRequest& rReq )
>                      if ( !bCreateNew )
>                          pPV->DeleteHelpLine(nHelpLine);
>                      // und weiter wie bei default
> +                    /*fall-through*/
>                  default:
>                      return;
>              }
> _______________________________________________
> LibreOffice mailing list
> LibreOffice@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/libreoffice