Ticket #7876 (closed: fixed)
Two hour time-boxed fix of issues shown up by Coverity Scan
Reported by: | Russell Taylor | Owned by: | Russell Taylor |
---|---|---|---|
Priority: | major | Milestone: | Release 3.0 |
Component: | Framework | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Tester: | Peter Peterson |
Description
Following the results of having mantid scanned at https://scan.coverity.com
Focus on high-impact defects, and in particular memory leaks in MantidPlot.
Change History
comment:2 Changed 7 years ago by Russell Taylor
Re #7876. Fix memory leak.
I'm not sure if this constructor actually can throw, but regardless there's no need to create it on the heap.
Changeset: 5972346a95068f611b2ef0ce489d73ce4dcef073
comment:3 Changed 7 years ago by Russell Taylor
Re #7876. Fix potential memory leaked.
This leaked if m was NULL at line 12178. Use RAII to avoid the problem.
Changeset: fccb57898c6f9b56507eb99b5a3a7d0696605aad
comment:4 Changed 7 years ago by Russell Taylor
Re #7876. Fix potential memory leaks.
Changeset: 332cf05d13b1cbbb5e8a6db5724a2e0d31bff2e7
comment:5 Changed 7 years ago by Russell Taylor
Re #7876. Use vectors instead of arrays to avoid leaks.
Changeset: b5e74fd8020b90d2e129eaeec6224e865bbb1639
comment:6 Changed 7 years ago by Russell Taylor
Re #7876. Fix memory leaks.
Don't create objects on the heap if you don't need to!
Changeset: 9569a2278c847571c3ab422efd9596838cb4cf64
comment:7 Changed 7 years ago by Russell Taylor
Re #7876. Correct to use array delete.
Changeset: 6d285825cce5f007c0215d36dc663997762cdb03
comment:8 Changed 7 years ago by Russell Taylor
Re #7876. Move delete until after last use.
m_data is used within saveSettings(), so we'd better not delete it until after that!
Changeset: f7597b976aed2b3f43183f134b9d6bb6bd7f6fc8
comment:9 Changed 7 years ago by Russell Taylor
Re #7876. Delete right before new allocation.
The coverity scan complained that the deleted fitter could be used at line 234 if all the if tests above it were false. Don't know if that can happen, but let's be safe.
Changeset: d93cba1198875707d46ae22cf9d25f3ca6bb5ead
comment:10 Changed 7 years ago by Russell Taylor
Re #7876. Remove unused method.
I don't think this method is used anywhere, which is a good thing as it was returning a pointer to a temporary.
Changeset: 05624096c6690e90613428d314883f9b124ee24f
comment:11 Changed 7 years ago by Russell Taylor
Re #7876. Refactor to not return pointers to temporaries.
Changeset: c82dbff9237d743d1067e9cc689c5bd11ae18d3c
comment:12 Changed 7 years ago by Russell Taylor
Re #7876. Make sure variable is initialised.
In 332cf05 I removed the call that also initialised the array. This remedies that.
Changeset: ee873ee04e7c9fa4b706381663f147791abb09c5
comment:13 Changed 7 years ago by Russell Taylor
Re #7876. Fix for compilers that don't do initializer lists.
Changeset: 7f6aefcfd43ab6c7451758badc519f2d7e15098e
comment:14 Changed 7 years ago by Russell Taylor
Re #7876. Remove compiler warning.
Hope that naming the variable prevents the compiler optimizing it away.
Changeset: 4ab08e5550b42677336447c328d7509ccb51cfce
comment:15 Changed 7 years ago by Russell Taylor
- Status changed from inprogress to verify
- Resolution set to fixed
Was more like 3 hours in the end, mainly because I decided to address some of invalid deletes, use-after-free errors & returning pointers to temporaries that are the errors that are likely to make Mantid crash.
The main way to test this is by inspection of the code changes, plus making sure that all tests are passing.
comment:16 Changed 7 years ago by Peter Peterson
- Status changed from verify to verifying
- Tester set to Peter Peterson
comment:17 Changed 7 years ago by Peter Peterson
- Status changed from verifying to closed
Merge remote branch 'origin/feature/7876_coverity_errors'
comment:18 Changed 5 years ago by Stuart Campbell
This ticket has been transferred to github issue 8721
Re #7876. Fix potential memory leak.
Don't leak memory if one of the 3 allocations failed.
Changeset: 4953b3777266b87c4b8c4a5681ee71ea8c84f738