Fixes crash, when removing last torrent.BUG: 388803
ClosedPublic
Actions

Authored by gassaf on Dec 2 2019, 11:15 AM.

Details

Reviewers

stikonas
trufanov

Commits

R473:91de705359dd: Fixes crash, when removing last torrent.BUG: 388803

Summary

Torrent wasn't removed from queuemanager properly. Thus calling the data(..) function caused ktorrent to crash.

Test Plan

Have one completed torrent in ktorrent
Remove this torrent
Ktorrent doesnot crash anymore

Diff Detail

Repository

R473 KTorrent

Branch

bugfix_388803 (branched from master)

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 19565
Build 19583: arc lint + arc unit

gassaf requested review of this revision.Dec 2 2019, 11:15 AM

gassaf created this revision.

Harbormaster completed remote builds in B19380: Diff 70732.Dec 2 2019, 11:15 AM

gassaf retitled this revision from Fixes crash, when removing last torrent. BUG: 388803 to Fixes crash, when removing last torrent.BUG: 388803.Dec 2 2019, 11:17 AM

gassaf edited the summary of this revision. (Show Details)

gassaf added reviewers: stikonas, trufanov.

Once I faced with this bug on Kubuntu 19.10 but couldn't reproduce. And I still can't reproduce it. Anyway change looks fine for me. Let's rely on stikonas opinion

ktorrent/tools/queuemanagermodel.cpp
152	I'm getting /tmp/D25680.diff:20: trailing whitespace. warning: 1 line adds whitespace errors. while git apply the diff bcs of this line. Could you fix?

stikonas added inline comments.Dec 2 2019, 7:37 PM

ktorrent/tools/queuemanagermodel.cpp
142	I am a bit reluctant to use foreach in new code. Firstly, of all they are discouraged (https://www.kdab.com/goodbye-q_foreach/), secondly, you have to introduce index variable `r` anyway. On the other hand it's very similar code to the code in other parts of this file... Other than that, it looks alright.

use for loop instead of foreach

Harbormaster completed remote builds in B19565: Diff 71037.Dec 7 2019, 12:28 AM

I use the for loop now and I have also removed that line with trailing white spaces.

stikonas requested changes to this revision.Dec 7 2019, 12:32 AM

stikonas added inline comments.

ktorrent/tools/queuemanagermodel.cpp
142	This might result in undefined behaviour. You are iterating over a container that is being modified. You need to take a copy here See section "Does the loop body (directly or indirectly) modify the container iterated over?" in https://www.kdab.com/goodbye-q_foreach/

This revision now requires changes to proceed.Dec 7 2019, 12:32 AM

Iterate over copy

Harbormaster completed remote builds in B19590: Diff 71074.Dec 8 2019, 12:16 AM

stikonas accepted this revision.Dec 8 2019, 12:24 AM

This revision is now accepted and ready to land.Dec 8 2019, 12:24 AM

Closed by commit R473:91de705359dd: Fixes crash, when removing last torrent.BUG: 388803 (authored by gassaf). · Explain WhyDec 8 2019, 12:37 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			ktorrent/tools/queuemanagermodel.cpp (13 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	70732	52a0eb1		Dec 2 2019, 11:15 AM	★	★
Diff 2	71037	52a0eb1	use for loop instead of foreach	Dec 7 2019, 12:28 AM	★	★
Diff 3	71074	52a0eb1	- Iterate over copy	Dec 8 2019, 12:16 AM	★	★
Diff 4	71075	8e0145d	R473:91de705359dd6bcfbb3613cf7dc13a8b835f27ac	Dec 8 2019, 12:36 AM	★	★

Commit	Tree	Parents	Author	Summary	Date
3b03ff48b7c5	6cb3ba493b86	2d53ada03a77	Gilbert Assaf	use for instaed of foreach	Dec 7 2019, 12:27 AM
2d53ada03a77	73b68dd86235	52a0eb17d142	Gilbert Assaf	Fixes crash, when removing last torrent. BUG: 388803 (Show More…)	Dec 2 2019, 11:11 AM

Diff 71037

View Options

ktorrent/tools/queuemanagermodel.cpp