Differential D14576

[Desktop Scripting ConfigGroup] Add more nullptr checks
ClosedPublic
Actions

Authored by broulik on Aug 3 2018, 8:47 AM.

Details

Reviewers
None
Group Reviewers
Plasma
Commits
R120:79ac08108966: [Desktop Scripting ConfigGroup] Add more nullptr checks
Summary

They were missing in a couple of places leading to crashes.

Test Plan

Ran the following script

print(new ConfigFile().groupList);

Used to crash, now it no longer does

Despite the qjsengine port, this file didn't change in 5.12, so should be easily backportable

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Aug 3 2018, 8:47 AM
Restricted Application added a project: Plasma. · View Herald TranscriptAug 3 2018, 8:47 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Aug 3 2018, 8:47 AM
broulik edited the test plan for this revision. (Show Details)
anthonyfieroni added a subscriber: anthonyfieroni.Aug 3 2018, 10:06 AM
anthonyfieroni added inline comments.
shell/scripting/configgroup.cpp
67

Here as well should be sync(), if it destroys object before timer is elapsed e.g. before 1.5 sec it will crash.

broulik updated this revision to Diff 39004.Aug 3 2018, 10:08 AM
  • Check in destructor as well
davidedmundson added a subscriber: davidedmundson.Aug 3 2018, 2:57 PM

KConfigGroup is internally shared.

Why have it on the heap in the first place?

broulik added a comment.Aug 3 2018, 5:55 PM

I didn't write that code, I noticed a crash that was trivially fixed and did just that

This revision was not accepted when it landed; it landed in state Needs Review.Aug 6 2018, 9:44 AM
Closed by commit R120:79ac08108966: [Desktop Scripting ConfigGroup] Add more nullptr checks (authored by broulik). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mshell/scripting/configgroup.cpp (9 lines)

Diff 39171

View Options

shell/scripting/configgroup.cpp

Loading...