Differential D18289

[gitpushgialog] Check index validity
ClosedPublic
Actions

Authored by anthonyfieroni on Jan 16 2019, 6:14 AM.

Details

Reviewers
elvisangelaccio
alexeymin
Group Reviewers
Dolphin
Commits
R449:9f8f6c152116: [gitpushdialog] Check GitWrapper::branches index validity
Summary

I test to push in repo that i don't have permissions, it crash before showing dialog. Now it show it.

Diff Detail

Repository
R449 Plugins for Dolphin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
anthonyfieroni requested review of this revision.Jan 16 2019, 6:14 AM
anthonyfieroni created this revision.
alexeymin accepted this revision.Jan 21 2019, 4:54 PM
alexeymin added a subscriber: alexeymin.

branches is a QStringList, so, we could also check that currentBranchIndex < branches.size() in condition, if it is all about bounds checking.
Anyway, the diff compiles and overall looks harmless. I'd say +1, I did not try to reproduce the crash though

This revision is now accepted and ready to land.Jan 21 2019, 4:54 PM
anthonyfieroni added a comment.Jan 21 2019, 7:18 PM
In D18289#397527, @alexeymin wrote:

branches is a QStringList, so, we could also check that currentBranchIndex < branches.size() in condition, if it is all about bounds checking.

In this case GitWrapper::branches sets valid index or -1, so it's not need bound check.

Closed by commit R449:9f8f6c152116: [gitpushdialog] Check GitWrapper::branches index validity (authored by anthonyfieroni). · Explain WhyJan 21 2019, 7:23 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mgit/pushdialog.cpp (4 lines)

Diff 50019

View Options

git/pushdialog.cpp

Loading...