This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103202/ |
On November 22nd, 2011, 9:24 a.m., Thomas Lübking wrote:
plasma/private/containment_p.h (Diff revision 1) public:71 while (!applets.isEmpty()) {ibeg your pardon? to me this looks like you delete the same entry forever (what *will* crash at a point) since you don't remove it - should be "takeFirst", yesno?On November 22nd, 2011, 9:27 a.m., Ben Cooksley wrote:
Other code handles this for us - which is why qDeleteAll() crashes.On November 22nd, 2011, 9:57 a.m., Thomas Lübking wrote:
sorry, but the only way i see this could be correct is that qlist (fun with templates) subclasses pointer Types and reimplements delete to automatically remove them from the list, what is, nicely spoken, sick. is there any detailed (official) explanation on this?On November 22nd, 2011, 10:11 a.m., Aaron J. Seigo wrote:
see ContainmentPrivate::appletDestroyed(Plasma::Applet *applet) in kdelibs/plasma/containment.cppOn November 22nd, 2011, 10:12 a.m., Ben Cooksley wrote:
Only information I have is https://git.reviewboard.kde.org/r/102981/ I haven't checked the code, but i'm guessing that Plasma::Applet must do something similar to what Akregator does. In any case, I tested and Plasma does not infinitely loop - it does close properly.
okey, thanks - that explains things. qlist needs a mutex (at least as friend for qdeleteall) - and imho this here needs another comment since esp. the superfluous clear() makes it look like i bug. (written by touch in a shaky train ;-)
- Thomas
On November 22nd, 2011, 7:39 a.m., Ben Cooksley wrote:
Review request for kdelibs and Plasma.
By Ben Cooksley.
Updated Nov. 22, 2011, 7:39 a.m. Description
Testing
Diffs
|