[Kde-pim] Review Request 111101: Use QPointer to store move|copy and selectfolder dialogs

[Christian Mollekopf]

> On June 19, 2013, 5:18 a.m., Laurent Montel wrote:
> > kmail/kmmainwidget.cpp, line 2147
> > <http://git.reviewboard.kde.org/r/111101/diff/3/?file=164317#file164317line2147>
> >
> >     I prefere a if (dialog->exec() && dialog) {... } it's more compact.
> >     
> >     and we use this style in other code.
> >     Thanks
> 
> Christian Mollekopf wrote:
>     The evaluation order is left to right, so you're first accessing the pointer and then checking if it's null, am I missing something?
> 
> Kevin Krammer wrote:
>     This is about checking the QPointer after the dialog returns from exec(), which runs a nested event loop and could in the worst case lead to the dialog being deleted while "it is running". In this case the QPointer will be reset to 0, so this additional check will make sure no attempt is made to retrieve values from it.
>

Ah, that makes sense. Thanks for the clarification.


- Christian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111101/#review34639
-----------------------------------------------------------


On June 18, 2013, 9:34 p.m., Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111101/
> -----------------------------------------------------------
> 
> (Updated June 18, 2013, 9:34 p.m.)
> 
> 
> Review request for KDEPIM and David Faure.
> 
> 
> Description
> -------
> 
> The code showing the move|copy and select folder dialogs tries to check the dialogs are not deleted while they are visible, but it actually recreates them instead of checking their existence. The patch changes this to use QPointer to track the dialogs instead.
> 
> 
> Diffs
> -----
> 
>   kmail/kmmainwidget.h 977e1e13a85d703f71c234697d5cf7fe72577843 
>   kmail/kmmainwidget.cpp 36e7164a58d59cd35f296b9ab6885f1de8c44572 
> 
> Diff: http://git.reviewboard.kde.org/r/111101/diff/
> 
> 
> Testing
> -------
> 
> Dialogs still show and work correctly.
> 
> 
> Thanks,
> 
> Aurélien Gâteau
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/