ngraham added inline comments. INLINE COMMENTS
> davidedmundson wrote in image.cpp:362 > there's two mistakes that cancel kinda each other out here. > > typically, this should be qAsConst(m_slidePaths) > m_slidePaths here detatches and does a full deep-copy > > https://www.dvratil.cz/2015/06/qt-containers-and-c11-range-based-loops/ > > BUT: > > you modify m_slidePaths whilst you are iterating through m_slidePaths. If we > hadn't accidentally detached this would be very crashy dangerous code. > > Feel free to claim it was deliberate because you are a genius. > > ----- > > However, rather than a deep copy ideally we would just force a shallow copy > i.e > > const QStringList preprocssedPaths = m_slidePaths; > for (const QString &path: preprocssedPaths) { > ... > } > > best of all worlds haha, in fact it was deliberate, but I'm hardly a genius, and your suggestion makes more sense. I'll do that. REPOSITORY R120 Plasma Workspace REVISION DETAIL https://phabricator.kde.org/D26483 To: ngraham, #plasma, #vdg, ndavis, davidedmundson Cc: davidre, broulik, davidedmundson, ndavis, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart