[Kde-pim] [patch] kdepimlibs/kabc: fix crash appearing at qDeleteAll ()

[Ingo Klöcker]

On Wednesday 18 June 2008, Jaroslaw Staniek wrote:
> Ingo Klöcker said the following, On 2008-06-18 20:50:
> > On Wednesday 18 June 2008, Jaroslaw Staniek wrote:
> >> (for review; enterprise4 branch)
> >>
> >> - Resource::clear() fix crash appearing at
> >> qDeleteAll(mDistListMap) beacuse DistributionList objects'
> >> destructor calls
> >>    Resource::removeDistributionList(), which in turn modifies with
> >>    mDistListMap. We are now blocking removeDistributionList() when
> >> qDeleteAll() is called.
> >>    Instead of erasing() every deleted item from the map, we're
> >> calling QMap::clear() afterwards.
> >>
> >> - simplify code related to mDistListMap: use Qt4 facilities and
> >> some more constness
> >
> > I propose the following solution that avoids adding a member
> > variable at the cost of copying mDistListMap:
>
> This is not the only cost.
> removeDistributionList() and mDistListMap.find() is also called for
> every item. Please look at the new patch -is that ok? :
>
> void Resource::removeDistributionList( DistributionList *list ) {
> 	if ( mDistListMap.isEmpty() )
> 		return;
> [..]

I don't think it's really needed. mDistListMap.find() isn't really 
expensive if mDistListMap is empty. Moreover, this code isn't really 
performance critical because the KABC resource surely isn't destroyed a 
thousand times in a tight loop. So let's avoid premature optimization 
and leave Resource::removeDistributionList() unchanged.


Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20080618/51ff2456/attachment.sig>
-------------- next part --------------
_______________________________________________
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/