Differential D13559

Fix some of cppcheck warnings
ClosedPublic
Actions

Authored by bshah on Jun 15 2018, 11:33 AM.

Details

Reviewers
aacid
Group Reviewers
Frameworks
Commits
R127:84bc3cbd6102: Fix some of cppcheck warnings
Summary
  • selfInitialization
  • memleak

I could probably suppress the selfInitialization warning, but probably
is cleaner this way. Opinions welcome. This also fixes the memleak in
autotest.

Test Plan
  • arc lint --everything
  • Fix issues
  • build
  • run tests

Diff Detail

Repository
R127 KWayland
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bshah created this revision.Jun 15 2018, 11:33 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptJun 15 2018, 11:33 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
bshah requested review of this revision.Jun 15 2018, 11:33 AM
apol added a subscriber: apol.Jul 1 2018, 11:46 PM

I don't see any fix here. It's just silencing cppcheck.

bshah added a comment.Jul 2 2018, 7:21 AM
In D13559#285804, @apol wrote:

I don't see any fix here. It's just silencing cppcheck.

Only supressing done is the memleak in the autotests, otherwise for selfInitialization it is the false alarm, which can be fixed by giving different name to argument variable.

aacid added a subscriber: aacid.Jul 2 2018, 11:00 PM
In D13559#285836, @bshah wrote:
In D13559#285804, @apol wrote:

I don't see any fix here. It's just silencing cppcheck.

Only supressing done is the memleak in the autotests, otherwise for selfInitialization it is the false alarm, which can be fixed by giving different name to argument variable.

Changing the variable names is a good thing, it makes it much easier to read even if the old code is still as valid as the new.

About the memory leak, unless it's really hard to fix, i'd prefer a fix, because you never know when a unittest would actually show up a memory leak so instead of just blanket ignoring them i think it makes sense to have tests to be "correct" on the memory handling department too.

bshah updated this revision to Diff 37087.Jul 3 2018, 3:17 AM
  • fix memory leak properly
bshah updated this revision to Diff 37088.Jul 3 2018, 3:18 AM
bshah edited the summary of this revision. (Show Details)
bshah removed subscribers: aacid, apol.

update commit message

bshah added subscribers: aacid, apol.Jul 3 2018, 3:19 AM

Added subscribers back which arc removed.

aacid accepted this revision.Jul 3 2018, 10:01 PM
This revision is now accepted and ready to land.Jul 3 2018, 10:01 PM
Closed by commit R127:84bc3cbd6102: Fix some of cppcheck warnings (authored by bshah). · Explain WhyJul 4 2018, 10:30 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 37135

View Options

autotests/server/test_display.cpp

Loading...
View Options

src/client/xdgoutput.cpp

Loading...
View Options

src/server/dpms_interface.cpp

Loading...
View Options

src/server/xdgoutput_interface.cpp

Loading...