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
Lint Skipped
Unit
Unit Tests Skipped
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 49593

View Options

git/pushdialog.cpp

Loading...