> On Dec. 18, 2015, 3:17 a.m., Alex Richardson wrote: > > src/lib/plugin/desktopfileparser.cpp, line 346 > > <https://git.reviewboard.kde.org/r/126409/diff/1/?file=424369#file424369line346> > > > > `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. > > Michael Pyne wrote: > I prefer to factor loop invariants (such as updated defs here) out of > if/else blocks, which is why I used temporary, but whichever fix is used will > be pretty obvious here so I've submitted a revision.
Any objections to the revised patch? - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126409/#review89679 ----------------------------------------------------------- On Dec. 18, 2015, 3:44 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, 3:44 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
