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

[Sergio Martins]

> On 2009-04-05 01:03:49, Kevin Krammer wrote:
> > trunk/KDE/kdepimlibs/kcal/resourcelocaldir.cpp, line 424
> > <http://reviewboard.kde.org/r/516/diff/2/?file=4933#file4933line424>
> >
> >     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)

It's a comparison, not an assignment


> On 2009-04-05 01:03:49, Kevin Krammer wrote:
> > trunk/KDE/kdepimlibs/kcal/resourcelocaldir_p.h, line 60
> > <http://reviewboard.kde.org/r/516/diff/2/?file=4934#file4934line60>
> >
> >     can probably be made const

Changing to const


> On 2009-04-05 01:03:49, Kevin Krammer wrote:
> > trunk/KDE/kdepimlibs/kcal/resourcelocaldir_p.h, line 59
> > <http://reviewboard.kde.org/r/516/diff/2/?file=4934#file4934line59>
> >
> >     this can probably be made static or at least const

Changing to static


> On 2009-04-05 01:03:49, Kevin Krammer wrote:
> > trunk/KDE/kdepimlibs/kcal/resourcelocaldir.cpp, line 187
> > <http://reviewboard.kde.org/r/516/diff/2/?file=4933#file4933line187>
> >
> >     Probably better to use foreach
> >     foreach ( const QString &entry, entries ) {
> >       if ( d->isTempFile( entry ) ) {
> >       }
> >     }

Using a foreach now


- Sergio


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


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/