Fixes for issues picked up by coverity
ClosedPublic

Authored by piggz on Dec 27 2016, 9:24 PM.

Details

Summary

Mostly uninitialised variables and non checks for dynamic_cast

Test Plan

Run kreportexample and test rendering

Diff Detail

Repository
R14 KReport
Branch
coverity
Lint
No Linters Available
Unit
No Unit Test Coverage
piggz updated this revision to Diff 9402.Dec 27 2016, 9:24 PM
piggz retitled this revision from to Fixes for issues picked up by coverity.
piggz updated this object.
piggz edited the test plan for this revision. (Show Details)
piggz added a reviewer: staniek.
Restricted Application added a project: KReport. · View Herald TranscriptDec 27 2016, 9:24 PM
staniek requested changes to this revision.Dec 28 2016, 12:03 AM
staniek edited edge metadata.

I propose much smaller change set, using approach known from KDb/KProperty/Kexi. Thanks and sorry for not discussing earlier.

Probably it will be easier for you to apply the patch to a new branch without commiting and selectively pick line changes e.g. using git-gui.

src/common/KReportItemBase.cpp
36

This is not needed. This class of warnings is a false positive. If you mark them as such at coverity.com we don't need any changes and next time it won't complain. Please note that setting nullptr here is misleading.

If you want you can change initialization from Private::Private() to an initializer list:

: set(new KPropertySet), ....
src/common/KReportItemLine.cpp
134–139

Good one.

src/common/KReportRenderObjects.cpp
182

not needed, false positive :/

291

this = nullptr not needed, false positive :/

src/renderer/KReportPage.cpp
45

not needed, it's properly initialized

48–49

not needed, I propose to mark as a false positive

src/renderer/scripting/KReportScriptHandler.cpp
46–47

not needed, I propose to mark all these as false positives

or 3 of them can be changed to initializers in Private()

src/wrtembed/KReportDesigner.cpp
123

I propose to mark false positives for members that are initialized in KReportDesigner::init() and don't initialize here.

Summing up, if we fake-initialize just to silent the Coverity here we can miss the fact that we forget to allocate an object when we change KReportDesigner::init(), it would not be noticed.

src/wrtembed/KReportDesignerItemBase.cpp
33 ↗(On Diff #9402)

I propose to mark both as false positives without initializing here.

src/wrtembed/KReportDesignerItemLine.cpp
173–178

Can we avoid the underscore?

src/wrtembed/KReportDesignerItemRectBase.cpp
115–120

Can we skip the _?

274–311

Same here

src/wrtembed/KReportDesignerItemRectBase.h
53–54

BTW, Hmm why KReportDpiSingleton::m_dpiX/Y == 75?

53–54

both not needed, KReportDesignerItemRectBase initializes them; I propose to mark false positives

55

Hmm are these 4 members used?

src/wrtembed/KReportDesignerSection.cpp
96

Not needed... all false positives please

392–404

Can we skip _?

src/wrtembed/KReportDesignerSectionDetail.cpp
40–41

Same here

src/wrtembed/KReportDesignerSectionDetailGroup.cpp
46–47

Same here

src/wrtembed/KReportDesignerSectionScene.h
70

Same here

This revision now requires changes to proceed.Dec 28 2016, 12:03 AM
piggz updated this revision to Diff 9530.Dec 30 2016, 11:03 PM
piggz edited edge metadata.
piggz marked 20 inline comments as done.
  • Simplify patch by removing unnescessary initialisation

I updated this review becuase it was easier to track the needed changes here, rather than in coverity, which has a really annoying interface....i cant get the panel of issues to display properly with all issues, and the code view below!

src/wrtembed/KReportDesignerItemRectBase.h
53–54

From what I gather, 96 is most common fall-back dpi, so lets use that

piggz updated this revision to Diff 9534.Dec 31 2016, 8:46 AM
piggz edited edge metadata.

Rebased on master

piggz added a comment.Dec 31 2016, 9:23 AM

Have now updated coverity to mark as false-positives

staniek requested changes to this revision.Dec 31 2016, 10:31 AM
staniek edited edge metadata.

Good. I do realize that removing can mean more work than adding :)

Happy 2k17!

src/common/KReportItemBase.cpp
38

not needed, it's initialized in the ctor

src/wrtembed/KReportDesignerItemLine.cpp
173–178

qobject_cast

src/wrtembed/KReportDesignerItemRectBase.cpp
115–120

qobject_cast

274–311

qobject_cast

src/wrtembed/KReportDesignerSection.cpp
96

Still, not needed, there's d->dpiY = KReportPrivate::dpiY(); code below

src/wrtembed/KReportDesignerSectionDetailGroup.cpp
32

+space

src/wrtembed/KReportDesignerSectionScene.h
73–77

I see that m_minorSteps is used in two KReportDesignerSectionScene methods. Proposed: move it to local variable from here.

src/wrtembed/KReportDesignerSectionView.h
43

Not needed, always set in the ctor, is this even a false positive?

This revision now requires changes to proceed.Dec 31 2016, 10:31 AM
piggz marked 7 inline comments as done.Dec 31 2016, 12:57 PM
piggz added inline comments.
src/wrtembed/KReportDesignerSectionView.h
43

possibly not, i was just trying to be consistent and set everything to nullptr :)

piggz marked an inline comment as done and an inline comment as not done.Dec 31 2016, 1:08 PM
piggz added inline comments.
src/wrtembed/KReportDesignerSectionDetailGroup.cpp
32

where would you like a space?

staniek added inline comments.Dec 31 2016, 1:17 PM
src/wrtembed/KReportDesignerSectionDetailGroup.cpp
32

() {}

piggz marked 2 inline comments as done and an inline comment as not done.Dec 31 2016, 2:37 PM
piggz added inline comments.
src/common/KReportItemBase.cpp
38

is it better to set to 0 the c++11 way, in Private() or in KReportItemBase() ?

staniek added inline comments.Dec 31 2016, 4:26 PM
src/common/KReportItemBase.cpp
38

The c++11 way in Private() if it is only possible. Here it is.

Most known exceptions are related to creating QWidgets/QObjects where we have to do that in ctor of the regular class.

piggz updated this revision to Diff 9558.Dec 31 2016, 4:30 PM
piggz edited edge metadata.
  • Address review comments
piggz marked 5 inline comments as done.Dec 31 2016, 4:32 PM
staniek accepted this revision.Dec 31 2016, 4:35 PM
staniek edited edge metadata.
This revision is now accepted and ready to land.Dec 31 2016, 4:35 PM
This revision was automatically updated to reflect the committed changes.