Mostly uninitialised variables and non checks for dynamic_cast
Details
- Reviewers
staniek - Commits
- R14:b7ee4ca8e5a1: Fixes for issues picked up by coverity
Run kreportexample and test rendering
Diff Detail
- Repository
- R14 KReport
- Branch
- coverity
- Lint
No Linters Available - Unit
No Unit Test Coverage
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 |
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 |
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? |
src/wrtembed/KReportDesignerSectionView.h | ||
---|---|---|
43 | possibly not, i was just trying to be consistent and set everything to nullptr :) |
src/wrtembed/KReportDesignerSectionDetailGroup.cpp | ||
---|---|---|
32 | where would you like a space? |
src/wrtembed/KReportDesignerSectionDetailGroup.cpp | ||
---|---|---|
32 | () {} |
src/common/KReportItemBase.cpp | ||
---|---|---|
38 | is it better to set to 0 the c++11 way, in Private() or in KReportItemBase() ? |
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. |