[Kde-pim] [PATCH] kmail recipient picker fixes

Sun Nov 11 00:17:25 GMT 2007

Hi Sander,

On Saturday 10 November 2007, Sander van Grieken wrote:
> This is the first time I'm posting to this list. I understand that this is
> the place to be if one wants to submit patches?
Yes, this is the right place.

> I've created a patch to kmail/recipientspicker that fixes crashes when
> receiving addressbook-changed events from kaddressbook, fixes duplicate
> resource and recipient entries and fixes some memory management problems.
Thanks for the patch! I tried it, but as soon as I open the recipient picker,
KMail crashes with the backtrace below (without the patch, it doesn't crash).
But just before that, I could see that the duplicated entries are indeed gone :)

Some things I noticed:
- What happens if a recipient has multiple e-mail addresses? According to the logic
  of genKey(), he will be added to the collection only once, since the key is made
  with the preferred e-mail address. However, there should be one entry for each 
  e-mail address. (I am not sure about the actual behavior since I couldn't test it)
- genKey() should be generateKey()
- You sometimes don't adhere to the (admittedly strange) coding style of KMail with
  regard to parenthesis. For example:
  > if (!mDistributionList.isEmpty())
  should be
  >if ( !mDistributionList.isEmpty() ) 
  or
  > mAllRecipients->addItem(*rcptIt);
  should be
  > mAllRecipients->addItem( *rcptIt );
  But this is minor, and you did it correctly in most places.

As for the crash, I think the reason is that a recipient can be in multiple collections.
For example, if you put a person in your address book into a category, he will be in two
collections, the one for the catogory and the one for the resource. This might be problematic,
because it looks like the code then double-deletes the items in ~RecipientsPicker().
But again, I am not sure and might be wrong, I didn't have time to look at this more toroughly.

Again, thanks for the work, I hope you'll come up with an improved version of the patch.

Regards,
Thomas

#0  0xb646f606 in q_atomic_decrement (ptr=0x1d1e1f1a) at /media/kdedev/home/qt-copy/include/QtCore/../../src/corelib/arch/qatomic_i386.h:97
#1  0xb646f67f in QBasicAtomic::deref (this=0x1d1e1f1a) at /media/kdedev/home/qt-copy/include/QtCore/../../src/corelib/thread/qatomic.h:93
#2  0xb6470674 in ~QList (this=0x876fff4) at /media/kdedev/home/qt-copy/include/QtCore/../../src/corelib/tools/qlist.h:528
#3  0xb64706b1 in ~QStringList (this=0x876fff4) at /media/kdedev/home/qt-copy/include/QtCore/../../src/corelib/tools/qstringlist.h:62
#4  0xb648489e in ~Private (this=0x876ff70) at /media/kdedev/home/build/KDE/kdepimlibs/kabc/addressee.cpp:101
#5  0xb6484c6f in ~QSharedDataPointer (this=0x87d9e58) at /media/kdedev/home/qt-copy/include/QtCore/../../src/corelib/tools/qshareddata.h:82
#6  0xb6478ea5 in ~Addressee (this=0x87d9e58) at /media/kdedev/home/build/KDE/kdepimlibs/kabc/addressee.cpp:154
#7  0xb79c3875 in ~RecipientItem (this=0x87d9e58) at /media/kdedev/home/src/KDE/kdepim/kmail/recipientspicker.h:50
#8  0xb79bcbf6 in RecipientsCollection::deleteAll (this=0x8756e70) at /media/kdedev/home/src/KDE/kdepim/kmail/recipientspicker.cpp:296
#9  0xb79bcd5b in RecipientsPicker::insertCollection (this=0x86d54c0, coll=0x86ea360) at /media/kdedev/home/src/KDE/kdepim/kmail/recipientspicker.cpp:571
#10 0xb79bf4ad in RecipientsPicker::insertAddressBook (this=0x86d54c0, addressbook=0x86eae20) 
at /media/kdedev/home/src/KDE/kdepim/kmail/recipientspicker.cpp:509
#11 0xb79bf673 in RecipientsPicker::qt_metacall (this=0x86d54c0, _c=QMetaObject::InvokeMetaMethod, _id=8, _a=0xbfe5f86c) 
at /media/kdedev/home/build/KDE/kdepim/kmail/recipientspicker.moc:200
#12 0xb7f2e998 in QMetaObject::activate (sender=0x86eae20, from_signal_index=4, to_signal_index=4, argv=0xbfe5f86c) at kernel/qobject.cpp:3080
#13 0xb7f2ee0d in QMetaObject::activate (sender=0x86eae20, m=0xb64c6420, local_signal_index=0, argv=0xbfe5f86c) at kernel/qobject.cpp:3142
#14 0xb6475ce2 in KABC::AddressBook::addressBookChanged (this=0x86eae20, _t1=0x86eae20) at /media/kdedev/home/build/KDE/kdepimlibs/kabc/addressbook.moc:102
#15 0xb6475dbe in KABC::AddressBook::resourceLoadingFinished (this=0x86eae20, resource=0x86f1ed8) 
at /media/kdedev/home/src/KDE/kdepimlibs/kabc/addressbook.cpp:930
#16 0xb6475ede in KABC::AddressBook::qt_metacall (this=0x86eae20, _c=QMetaObject::InvokeMetaMethod, _id=5, _a=0xbfe5fdbc) 
at /media/kdedev/home/build/KDE/kdepimlibs/kabc/addressbook.moc:88
#17 0xb7f2e998 in QMetaObject::activate (sender=0x86f1ed8, from_signal_index=4, to_signal_index=4, argv=0xbfe5fdbc) at kernel/qobject.cpp:3080
#18 0xb7f2ee0d in QMetaObject::activate (sender=0x86f1ed8, m=0xb64c6c70, local_signal_index=0, argv=0xbfe5fdbc) at kernel/qobject.cpp:3142
#19 0xb649a0c4 in KABC::Resource::loadingFinished (this=0x86f1ed8, _t1=0x86f1ed8) at /media/kdedev/home/build/KDE/kdepimlibs/kabc/resource.moc:87
#20 0xb1cb9732 in KABC::ResourceFile::emitLoadingFinished (this=0x86f1ed8) at /media/kdedev/home/src/KDE/kdepimlibs/kabc/plugins/file/resourcefile.cpp:286
#21 0xb1cb97a6 in KABC::ResourceFile::qt_metacall (this=0x86f1ed8, _c=QMetaObject::InvokeMetaMethod, _id=0, _a=0xbfe5fe7c) 
at /media/kdedev/home/build/KDE/kdepimlibs/kabc/plugins/file/resourcefile.moc:68
#22 0xb7f2e998 in QMetaObject::activate (sender=0x86f1ec8, from_signal_index=4, to_signal_index=4, argv=0x0) at kernel/qobject.cpp:3080
#23 0xb7f2ee0d in QMetaObject::activate (sender=0x86f1ec8, m=0xb7fa16e4, local_signal_index=0, argv=0x0) at kernel/qobject.cpp:3142
#24 0xb7f39bd5 in QSingleShotTimer::timeout (this=0x86f1ec8) at .moc/debug-shared/qtimer.moc:73
#25 0xb7f39c22 in QSingleShotTimer::timerEvent (this=0x86f1ec8) at kernel/qtimer.cpp:298
#26 0xb7f2f5ea in QObject::event (this=0x86f1ec8, e=0xbfe60864) at kernel/qobject.cpp:1088
#27 0xb59d6961 in QApplicationPrivate::notify_helper (this=0x806a0f8, receiver=0x86f1ec8, e=0xbfe60864) at kernel/qapplication.cpp:3558
#28 0xb59d6c62 in QApplication::notify (this=0xbfe60b7c, receiver=0x86f1ec8, e=0xbfe60864) at kernel/qapplication.cpp:3117
#29 0xb6be0093 in KApplication::notify (this=0xbfe60b7c, receiver=0x86f1ec8, event=0xbfe60864) 
at /media/kdedev/home/src/KDE/kdelibs/kdeui/kernel/kapplication.cpp:319
#30 0xb7f1d20e in QCoreApplication::notifyInternal (this=0xbfe60b7c, receiver=0x86f1ec8, event=0xbfe60864) at kernel/qcoreapplication.cpp:532
#31 0xb7f205dd in QCoreApplication::sendEvent (receiver=0x86f1ec8, event=0xbfe60864) 
at ../../include/QtCore/../../src/corelib/kernel/qcoreapplication.h:202
#32 0xb7f4993d in QTimerInfoList::activateTimers (this=0x806ceb4) at kernel/qeventdispatcher_unix.cpp:556
#33 0xb7f4688c in timerSourceDispatch (source=0x806ce80) at kernel/qeventdispatcher_glib.cpp:157
#34 0xb5289df2 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#35 0xb528cdcf in ?? () from /usr/lib/libglib-2.0.so.0
#36 0x0806c490 in ?? ()
#37 0x00000000 in ?? ()

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