Differential D13043

Properly sanitize input
ClosedPublic
Actions

Authored by apol on May 22 2018, 4:40 PM.

Details

Reviewers
broulik
Group Reviewers
Frameworks
Commits
R275:958e7c9c56d9: Properly sanitize input
Summary

Helps to use it together with ModelTest

Diff Detail

Repository
R275 KItemModels
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
apol created this revision.May 22 2018, 4:40 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMay 22 2018, 4:40 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
apol requested review of this revision.May 22 2018, 4:40 PM
broulik added a subscriber: broulik.May 23 2018, 9:03 AM
broulik added inline comments.
src/kconcatenaterowsproxymodel.cpp
70

Coding style, braces also for single line statements

116

You can also use {} here, no? Also, I would prefer an early return rather than unary operator

155–160

Coding style. Also, check parent.isValid() return {}; since this is a flat model?

apol marked 3 inline comments as done.May 25 2018, 12:09 PM
apol added inline comments.
src/kconcatenaterowsproxymodel.cpp
116

No, the ternary operator doesn't allow using {} saying it can't infer the type. Can change to an if.

155–160

why is it flat?

apol updated this revision to Diff 34858.May 25 2018, 12:10 PM
apol marked 2 inline comments as done.

address kai's remarks

broulik accepted this revision.May 29 2018, 1:23 PM
This revision is now accepted and ready to land.May 29 2018, 1:23 PM
Closed by commit R275:958e7c9c56d9: Properly sanitize input (authored by apol). · Explain WhyMay 29 2018, 1:25 PM
This revision was automatically updated to reflect the committed changes.
dfaure added a subscriber: dfaure.Jun 19 2018, 1:12 PM

BTW, the commit message refers to ModelTest, but that's the old modeltest.cpp, not the new QAbstractItemModelTester, right?

I'm working on QConcatenateTablesProxyModel (https://codereview.qt-project.org/166323) and there QAbstractItemModelTester passes with asserts for all these invalid calls. The old modeltest is now considered wrong ;)

Not saying this commit should be reverted or anything, this is just to let you know about QAbstractItemModelTester and the fact that the new idea is narrow contract, asserting is good, while the old modeltest assumed very tolerant model implementations.

apol added a comment.Jun 19 2018, 11:14 PM

Yep, this was from the old and deprecated modeltest.cpp.

I'll have to look into how to do it right :P

Revision Contents
Changeset List

PathPackages
Msrc/kconcatenaterowsproxymodel.cpp (16 lines)

Diff 35111

View Options

src/kconcatenaterowsproxymodel.cpp

Loading...