Details
- Reviewers
tbaumgart - Group Reviewers
KMyMoney - Commits
- R261:fd5405aefaf8: Coverity fixes
Diff Detail
- Repository
- R261 KMyMoney
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Generally it looks good to me though.
| kmymoney/converter/tests/matchfinder-test.cpp | ||
|---|---|---|
| 32 | Remove blank line | |
| kmymoney/dialogs/investactivities.cpp | ||
| 119 | Use Q_LIKELY and Q_UNLIKELY in these condition settings. See http://doc.qt.io/qt-5/qtglobal.html#Q_LIKELY for details. I only mention it here once, but it applies to many places in this patch. | |
| 120 | I am not sure, if it makes sense to return true in case the widget is missing. This was not covered by the original code, but it seems better to returns false here IMHO. | |
| kmymoney/mymoney/mymoneyobject.cpp | ||
| 29 | I thought I removed those unused methods already in commit 3db841bcd65274f12eaf379a09425bf3bd8e336d | |
| kmymoney/mymoney/tests/mymoneyfile-test.cpp | ||
| 53 | remove blank line | |
| kmymoney/plugins/kbanking/kbanking.cpp | ||
| 1321 | why did you remove this code? | |
| kmymoney/reports/tests/reportstestcommon.cpp | ||
| 482 | Removing this statement does not make sense! | |
| kmymoney/dialogs/investactivities.cpp | ||
|---|---|---|
| 119 | What's the use of optimizing code for speed for UI elements? Secondly, you can have something like that if (auto cat = dynamic_cast<KMyMoneyCategory*>(haveWidget("asset-account")))and it's unlikely that you want to wrap it in Q_LIKELY macro. | |
| 120 | Right. | |
| kmymoney/mymoney/mymoneyobject.cpp | ||
| 29 | You pointed to the wrong commit and no, you didn't remove all unused methods. | |
| kmymoney/plugins/kbanking/kbanking.cpp | ||
| 1321 | Because const GWEN_TIME *startTime = 0; so conditions check always evaluate to the first choice. | |
| kmymoney/reports/tests/reportstestcommon.cpp | ||
| 482 | Right, but searchHTML isn't used anywhere anyway. | |
| kmymoney/dialogs/investactivities.cpp | ||
|---|---|---|
| 119 | True, never mind. | |
| kmymoney/mymoney/mymoneyobject.cpp | ||
| 29 | Ah, sorry, I must have been confused. Then simply remove and don't just comment out or is there a reason to keep it? | |
| kmymoney/plugins/kbanking/kbanking.cpp | ||
| 1321 | True, but then you could also eliminate the if statement completely. | |
| kmymoney/reports/tests/reportstestcommon.cpp | ||
| 482 | Why don't you remove it completely then? Removing this one line is simply misleading the reader. | |
| kmymoney/plugins/kbanking/kbanking.cpp | ||
|---|---|---|
| 1321 | I think it was here on purpose. Maybe someone revising it in the future will decide it's needed and if I would remove it, the he would have no clue. | |
| kmymoney/reports/tests/reportstestcommon.cpp | ||
| 482 | It's used in some commented out code and maybe someone, someday would like to know how searchhtml looked like. | |