----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126409/#review89679 -----------------------------------------------------------
src/lib/plugin/desktopfileparser.cpp (line 346) <https://git.reviewboard.kde.org/r/126409/#comment61418> `defs << *def // This must...` instead? And then the `defs << *def` in the else block? Would remove the need for the temporary variable declaration at the top. Switching the if/else would also make this easier to understand. - Alex Richardson On Dec. 18, 2015, 12:49 a.m., Michael Pyne wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126409/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2015, 12:49 a.m.) > > > Review request for KDE Frameworks and Alex Richardson. > > > Repository: kcoreaddons > > > Description > ------- > > Patch to fix a use-after-free issue in > kcoreaddons/src/lib/plugin/desktopfileparser.cpp, noted by Coverity (CID > 1336157) > > The issue is that if `defs << *def` happens after the call to > s_serviceTypes->insert(), the `*def` might already have been deleted by > QCache. Attached patch fixes by making the copy before the call to insert(). > > Really, it's probably better to not use QCache here if we can avoid it, but > I'm not sure enough of the code to go that route. But if this is something > that's only run once or twice anyways then there's no real need to use QCache > instead of QMap, I would think. > > > Diffs > ----- > > src/lib/plugin/desktopfileparser.cpp 06a4a1d > > Diff: https://git.reviewboard.kde.org/r/126409/diff/ > > > Testing > ------- > > desktoptojsontest still passes, code compiles. > > > Thanks, > > Michael Pyne > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
