Fixes leak of DolphinFileMetaDataWidget in ToolTipManager. The
destructor of ToolTipManager failed to delete the m_fileMetaDataWidget
member. This is seen at shutdown but also when you close a tab that has
displayed a tooltip.
Details
Details
- Reviewers
elvisangelaccio - Group Reviewers
Dolphin - Commits
- R318:94d7e1471e0a: Fixes leak of DolphinFileMetaDataWidget in ToolTipManager
Compile Dolphin with address sanitizer
Open Dolphin
Show a tooltip
Close Dolphin
Diff Detail
Diff Detail
- Repository
- R318 Dolphin
- Branch
- fixes_leak_of_dolphin_file_meta_data_widget (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 9425 Build 9443: arc lint + arc unit
| src/views/tooltips/tooltipmanager.cpp | ||
|---|---|---|
| 85–87 | Why reset() it twice? The second reset() call should be enough | |
| src/views/tooltips/tooltipmanager.cpp | ||
|---|---|---|
| 85–87 | I was just trying to mimic the original behavior. If there is only a single reset then there will be a short period of time where two DolphinFileMetaDataWidget instances are allocated because first the new DolphinFileMetaDataWidget is allocated, then it it passed to the reset function, which deletes the instance it owns and keeps the new one. A standard way of avoiding this is simply to call reset first without any arguments, then the original instance is deleted before the new one is allocated. But I can change it to a single reset call, what do you think? | |
| src/views/tooltips/tooltipmanager.cpp | ||
|---|---|---|
| 85–87 |
Hmm, this feels like a theoretical problem only. IMHO readability and consistency of code should be preferred (we don't reset a QScopedPointer like this anywhere else in dolphin). | |
| src/views/tooltips/tooltipmanager.cpp | ||
|---|---|---|
| 85–87 | I have removed the extra reset to keep things consistent :) | |
| src/views/tooltips/tooltipmanager.cpp | ||
|---|---|---|
| 88 | QScopedPointer::get() is Qt 5.11+ only, please use QScopedPointer::data() instead. | |
| src/views/tooltips/tooltipmanager.cpp | ||
|---|---|---|
| 88 | Good catch ;) I have changed this to QScopedPointer::data() | |
| src/views/tooltips/tooltipmanager.cpp | ||
|---|---|---|
| 211 | What about this one? ;) | |
| src/views/tooltips/tooltipmanager.cpp | ||
|---|---|---|
| 211 | Sorry about that, fixed now ;) | |