This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R120:79ac08108966: [Desktop Scripting ConfigGroup] Add more
nullptr checks (authored by broulik).
REPOSITORY
R120 Plasma
broulik added a comment.
I didn't write that code, I noticed a crash that was trivially fixed and did
just that
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D14576
To: broulik, #plasma
Cc: davidedmundson, anthonyfieroni, plasma-devel, ragreen, Pitel, Zren
davidedmundson added a comment.
KConfigGroup is internally shared.
Why have it on the heap in the first place?
REPOSITORY
R120 Plasma Workspace
REVISION DETAIL
https://phabricator.kde.org/D14576
To: broulik, #plasma
Cc: davidedmundson, anthonyfieroni, plasma-devel, ragreen, Pitel, Z
broulik updated this revision to Diff 39004.
broulik added a comment.
- Check in destructor as well
REPOSITORY
R120 Plasma Workspace
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D14576?vs=39003&id=39004
REVISION DETAIL
https://phabricator.kde.org/D14576
AFFECTED FILES
shell
anthonyfieroni added inline comments.
INLINE COMMENTS
> configgroup.cpp:67
> d->synchTimer->stop();
> d->configGroup->sync();
> }
Here as well should be sync(), if it destroys object before timer is elapsed
e.g. before 1.5 sec it will crash.
REPOSITORY
R120 Plasma Work
broulik created this revision.
broulik added a reviewer: Plasma.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
broulik requested review of this revision.
REVISION SUMMARY
They were missing in a couple of places leading to crashes.
TEST