[Kde-pim] Review Request: Fix crashes in resource local dir

[Kevin Krammer]

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



trunk/KDE/kdepimlibs/kcal/resourcelocaldir.cpp
<http://reviewboard.kde.org/r/516/#comment509>

    Not sure you actually need this, the resource has a deleteIncidence() method which does that internally



trunk/KDE/kdepimlibs/kcal/resourcelocaldir.cpp
<http://reviewboard.kde.org/r/516/#comment510>

    Have you tested the case where a file is moved over the existing file?
    
    I vaguely remember a case where such an operation resulted in a created() rather than dirty().
    
    



trunk/KDE/kdepimlibs/kcal/resourcelocaldir.cpp
<http://reviewboard.kde.org/r/516/#comment505>

    Probably better to use foreach
    foreach ( const QString &entry, entries ) {
      if ( d->isTempFile( entry ) ) {
      }
    }



trunk/KDE/kdepimlibs/kcal/resourcelocaldir.cpp
<http://reviewboard.kde.org/r/516/#comment506>

    Common mistake: this only assigns the Incidence parts of the object, e.g. nothing subtype specific.
    Use KCal::AssignmentVisitor instead.
    (e.g. look how kdepim/akonadi/resources/ical does it)



trunk/KDE/kdepimlibs/kcal/resourcelocaldir_p.h
<http://reviewboard.kde.org/r/516/#comment507>

    this can probably be made static or at least const



trunk/KDE/kdepimlibs/kcal/resourcelocaldir_p.h
<http://reviewboard.kde.org/r/516/#comment508>

    can probably be made const


- Kevin


On 2009-04-04 12:18:01, Sergio Martins wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/516/
> -----------------------------------------------------------
> 
> (Updated 2009-04-04 12:18:01)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Resource local dir is a calendar resource that uses a separate file for each incidence. It uses a KDirWatch to listen for alterations made by external applications and tells KOrganizer to reload the incidences. Whenever KDirWatch sees an alteration, resource local dir closes and reopens the calendar, freeing all incidences, which are still in use by Korganizer, so it crashes.
> 
> Furthermore, KDirWatch also detects changes made by KOrganizer itself, so Korganizer ends up redrawing stuff it already has.
> 
> I changed resource local dir so it doesn't close and reopen the whole calendar for each incidence, instead I just add/remove/update the incidence in the calendar.
> 
> Basically the important parts of the patch are the slots that catch signals from KDirWatch, the rest is just refactoring to preserve BC.
>     
> void updateIncidenceInCalendar( const QString &file );
> void addIncidenceToCalendar( const QString &file );
> void deleteIncidenceFromCalendar( const QString &file );
> 
> 
> resourcelocaldir.h remains untouched, so I guess this is BC.
> 
> 
> This addresses bugs 180221 and 187595.
>     https://bugs.kde.org/show_bug.cgi?id=180221
>     https://bugs.kde.org/show_bug.cgi?id=187595
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepimlibs/kcal/resourcelocaldir_p.h 946603 
>   trunk/KDE/kdepimlibs/kcal/resourcelocaldir.cpp 946603 
> 
> Diff: http://reviewboard.kde.org/r/516/diff
> 
> 
> Testing
> -------
> 
> Made alterations/deletions/additions with KOrganizer.
> 
> Made alterations/deletions/additions directly with an editor to see if KOrganizer updated automatically.
> 
> 
> Thanks,
> 
> Sergio
> 
>

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