Fix occasional abort in the krunner plugin
ClosedPublic

Authored by fvogt on Jul 15 2017, 3:51 PM.

Details

Summary

Sometimes the DBus reply has no arguments, so avoid trying to access them.

Test Plan

No crashes so far.

Diff Detail

Repository
R856 Plasma Browser Integration
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
fvogt created this revision.Jul 15 2017, 3:51 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 15 2017, 3:51 PM

This shouldn't be needed.
Even an empty QList<QVariant> will have an argument of that empty QList<QVariant> and errors will have a different type.

It implies something is wrong elsewhere.

fvogt added a comment.Jul 16 2017, 9:22 AM

This shouldn't be needed.
Even an empty QList<QVariant> will have an argument of that empty QList<QVariant> and errors will have a different type.

I thought so too, until I got crashes without the test. IMO it can't hurt to prevent crashes due to misbehaviour of other components anyway.

Can't really say if there's something wrong elsewhere, but FWIW this patch indeed fixes crashing krunner, see https://bugs.kde.org/show_bug.cgi?id=382521 for the backtrace.

broulik edited edge metadata.Jul 27 2017, 1:02 PM

@davidedmundson Should we go with this now?

tabsrunner/tabsrunner.cpp
103

Perhaps even check != 1 - it shouldn't have more than one argument either.

broulik accepted this revision.Aug 1 2017, 10:14 AM
This revision is now accepted and ready to land.Aug 1 2017, 10:14 AM
This revision was automatically updated to reflect the committed changes.