> On April 23, 2014, 3:03 p.m., Thomas Lübking wrote: > > src/kstartupinfo.cpp, line 413 > > <https://git.reviewboard.kde.org/r/117701/diff/1/?file=267866#file267866line413> > > > > this is somehow not equivalent to the other variant (ignores > > silent/uninited) - the iterator should perhaps go with a map > > pointer/reference to stress this (and as a matching map usage is required > > anyway) > > Martin Gräßlin wrote: > my understand is that silent/unintended was dead code for these cases as > it only iterated over the startups and that if it's in startups it's not in > silent or unintended. I'll re-check that. > > Martin Gräßlin wrote: > yep, unintended and ignored do not matter in that case as it's either in > startups or in silent_startups or in unintended_startups: > > if (update_P) { // change: without any new: first > //qDebug() << "adding uninited"; > uninited_startups.insert(id_P, data_P); > } else if (data_P.silent() != Data::Yes || flags & > AnnounceSilenceChanges) { > //qDebug() << "adding"; > startups.insert(id_P, data_P); > emit q->gotNewStartup(id_P, data_P); > } else { // new silenced, and silent shouldn't be announced > //qDebug() << "adding silent"; > silent_startups.insert(id_P, data_P); > }
But yes, it might be a good idea to either rename the new method or add the map as a reference. Though I think rename is better. - Martin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117701/#review56269 ----------------------------------------------------------- On April 23, 2014, 11:35 a.m., Martin Gräßlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/117701/ > ----------------------------------------------------------- > > (Updated April 23, 2014, 11:35 a.m.) > > > Review request for KDE Frameworks, Àlex Fiestas and Bhushan Shah. > > > Bugs: 332681 > https://bugs.kde.org/show_bug.cgi?id=332681 > > > Repository: kwindowsystem > > > Description > ------- > > Fix crashers in KStartupInfo on remove while iterating > > This is similar to the change ... just for more cases. If the API was > used with ::checkStartup it could happen that an item got removed from > the startups while iterating the list of startups. Thus the data > corrupted resulting in a crash when getting a similar crash. > > In this case the code now uses erase instead of remove to have a valid > iterator. > > BUG: 332681 > > > Diffs > ----- > > autotests/kstartupinfo_unittest.cpp > 29fa320bc2c82e7e04a7322111bcdba44b7078c6 > src/kstartupinfo.cpp 6a95ce2d2eb79abbfbd072fa922f458da30d37eb > > Diff: https://git.reviewboard.kde.org/r/117701/diff/ > > > Testing > ------- > > See new unit tests which crash without the change. For people wanting to run > the test: be warned it has the power to crash plasma-shell and kwin. > > > Thanks, > > Martin Gräßlin > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
