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

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

Details

Summary

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

Test Plan
  1. Have one completed torrent in ktorrent
  2. Remove this torrent
  3. Ktorrent doesnot crash anymore

Diff Detail

Repository
R473 KTorrent
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
gassaf requested review of this revision.Mon, Dec 2, 11:15 AM
gassaf created this revision.
gassaf retitled this revision from Fixes crash, when removing last torrent. BUG: 388803 to Fixes crash, when removing last torrent.BUG: 388803.Mon, Dec 2, 11:17 AM
gassaf edited the summary of this revision. (Show Details)
gassaf added reviewers: stikonas, trufanov.
trufanov resigned from this revision.Mon, Dec 2, 5:25 PM

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.Mon, Dec 2, 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.

gassaf updated this revision to Diff 71037.Sat, Dec 7, 12:28 AM

use for loop instead of foreach

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

stikonas requested changes to this revision.Sat, Dec 7, 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.Sat, Dec 7, 12:32 AM
gassaf updated this revision to Diff 71074.Sun, Dec 8, 12:16 AM
  • Iterate over copy
stikonas accepted this revision.Sun, Dec 8, 12:24 AM
This revision is now accepted and ready to land.Sun, Dec 8, 12:24 AM
This revision was automatically updated to reflect the committed changes.