[Kde-pim] Review Request: folder properties: do not create a job on the stack

Kevin Krammer krammer at kde.org
Fri Nov 30 09:11:25 GMT 2012


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



kmail/kmmainwidget.cpp
<http://git.reviewboard.kde.org/r/107521/#comment17403>

    maybe slotCollectionPropertiesFinished?
    
    so the order of calls would be
    shoCollectionProperties, slotCollectionPropertiesContinued, slotCollectionPropertiesFinished



kmail/kmmainwidget.cpp
<http://git.reviewboard.kde.org/r/107521/#comment17404>

    I think a kWarning or kError logging and a return would be better.
    Since this is asynchronous the collection could have been removed elsewhere.
    The assert on job type is OK, because this is what we connect to the slot, i.e. something the code here has full control over


- Kevin Krammer


On Nov. 30, 2012, 8:36 a.m., Jonathan Marten wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107521/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2012, 8:36 a.m.)
> 
> 
> Review request for KDEPIM.
> 
> 
> Description
> -------
> 
> While reviewing https://git.reviewboard.kde.org/r/107341/, Kevin Krammer observed that a job was being created on the stack (by already existing code).  Since jobs auto-delete themselves by default, this is incorrect and could lead to a crash as a result of a double deletion.
> 
> This change creates the job on the heap instead, allowing it to delete itself when appropriate.
> 
> 
> Diffs
> -----
> 
>   kmail/kmmainwidget.h 5eefaa7 
>   kmail/kmmainwidget.cpp 5568b9a 
> 
> Diff: http://git.reviewboard.kde.org/r/107521/diff/
> 
> 
> Testing
> -------
> 
> Built kdepim with this change, checked operation of "Folder Properties" dialogue for both local and IMAP folders.
> 
> 
> Thanks,
> 
> Jonathan Marten
> 
>

_______________________________________________
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/



More information about the kde-pim mailing list