fix crash when mapping unified mailboxes to collections
ClosedPublic

Authored by jaham on Feb 25 2019, 11:14 PM.

Details

Summary

when the fetched unified mailbox collections and loaded mailboxes differ then
trying to get a loaded mailbox using the collection name may fail.
wrapping the call in a try-catch block fixes the crash

Diff Detail

Repository
R206 KMail
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jaham created this revision.Feb 25 2019, 11:14 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptFeb 25 2019, 11:14 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
jaham requested review of this revision.Feb 25 2019, 11:14 PM
mlaurent requested changes to this revision.Feb 26 2019, 6:28 AM
mlaurent added inline comments.
agents/unifiedmailboxagent/unifiedmailboxmanager.cpp
391–396

We don't use exception in kde apps in general
why don't use

std::unordered_map<....>::const_iterator got = mymap.find(col.name());
       if (got ==mMainBoxes.end()) {
            qWarning()...
        } else {
          .... setCollectionId(...)
        }
This revision now requires changes to proceed.Feb 26 2019, 6:28 AM
jaham added inline comments.Feb 26 2019, 7:59 PM
agents/unifiedmailboxagent/unifiedmailboxmanager.cpp
391–396

Normally i would do that too. But there is a similar try-catch block in the function UnifiedMailboxManager::createDefaultBoxes. I just wanted to follow the coding style already used.

jaham added inline comments.Feb 26 2019, 10:43 PM
agents/unifiedmailboxagent/unifiedmailboxmanager.cpp
391–396

Do you still want me to change it?

Yep I prefer.
I will create a patch for removing it in UnifiedMailboxManager::createDefaultBoxes
I prefer to not using exception when it's not really necessary

jaham updated this revision to Diff 52774.Feb 27 2019, 9:31 PM
  • use unordered_map::find to avoid dealing with exceptions
mlaurent accepted this revision.Feb 28 2019, 12:23 PM

Thanks
Do you have commit access ?
If not I can commit for you.

This revision is now accepted and ready to land.Feb 28 2019, 12:23 PM
This revision was automatically updated to reflect the committed changes.

Thanks for reviewing.
Looking at BKO I stumbled over bug #404347 which may be related. I did have a similar behaviour while the unified mailbox agent still crashed.