Possible null pointer dereference in sw/source/ui/misc/redlndlg.cxx

classic Classic list List threaded Threaded
2 messages Options
julien2412 julien2412
Reply | Threaded
Open this post in threaded view
|

Possible null pointer dereference in sw/source/ui/misc/redlndlg.cxx

Hello,

On master sources updated today, cppcheck reported this :
[sw/source/ui/misc/redlndlg.cxx:402]: (error) Possible null pointer dereference: pView - otherwise it is redundant to check if pView is null at line 406

    395 void SwRedlineAcceptDlg::Activate()
    396 {
    397     // prevent update if flag is set (#102547#)
    398     if( bInhibitActivate )
    399         return;
    400
    401     SwView *pView = ::GetActiveView();
    402     SwWait aWait( *pView->GetDocShell(), sal_False );
    403
    404     aUsedSeqNo.Remove((sal_uInt16)0, aUsedSeqNo.Count());
    405
    406     if (!pView) // can happen when switching to another app, when a Listbox in the dialog
    407         return; // had the focus previously (actually THs Bug)

Should we test pView just after having tested bInhibitActivate or should we keep this :
aUsedSeqNo.Remove((sal_uInt16)0, aUsedSeqNo.Count());
before testing pView ?

Julien
Michael Stahl-2 Michael Stahl-2
Reply | Threaded
Open this post in threaded view
|

Re: Possible null pointer dereference in sw/source/ui/misc/redlndlg.cxx

On 14/07/12 01:49, julien2412 wrote:

> Hello,
>
> On master sources updated today, cppcheck reported this :
> [sw/source/ui/misc/redlndlg.cxx:402]: (error) Possible null pointer
> dereference: pView - otherwise it is redundant to check if pView is null at
> line 406
>
>     395 void SwRedlineAcceptDlg::Activate()
>     396 {
>     397     // prevent update if flag is set (#102547#)
>     398     if( bInhibitActivate )
>     399         return;
>     400
>     401     SwView *pView = ::GetActiveView();
>     402     SwWait aWait( *pView->GetDocShell(), sal_False );
>     403
>     404     aUsedSeqNo.Remove((sal_uInt16)0, aUsedSeqNo.Count());
>     405
>     406     if (!pView) // can happen when switching to another app, when a
> Listbox in the dialog
>     407         return; // had the focus previously (actually THs Bug)
>
> Should we test pView ... ?

hmm... it appears that this code is essentially unchanged since the
initial import, except that the comment "can happen when... " there has
been translated from the original Klingon.

so pView ultimately comes from SfxViewFrame::Current(), dynamically
casted to SwView... i bet the idea is that null pView is never supposed
to happen, and the comment indicates that a certain "TH" was at some
time tasked with fixing the bug that it could indeed be null at that time.

since it has been unconditionally dereferenced for the last 12 years at
least anyway, i think the check is just redundant and could be removed.

_______________________________________________
LibreOffice mailing list
[hidden email]
http://lists.freedesktop.org/mailman/listinfo/libreoffice