----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126507/#review90439 -----------------------------------------------------------
src/kdeui/k4style.cpp (line 1176) <https://git.reviewboard.kde.org/r/126507/#comment61851> The double-lookup looks slow and unnecessary. contains + *object doesn't give you anything that object (followed by a dereference if not null) gives you. If the fear is concurrent access, then contains + *object can still dereference a null pointer anyway. But K4Style runs in the GUI thread always anyway. So I would call object(), put that into a pointer, and then if not null, make a copy into a value. This avoids the double lookup. - David Faure On Jan. 2, 2016, 1:55 a.m., Michael Pyne wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126507/ > ----------------------------------------------------------- > > (Updated Jan. 2, 2016, 1:55 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kdelibs4support > > > Description > ------- > > Fix a couple of Coverity issues: > > 1. CID 1175508; file descriptors used in KLockFile could be leaked in > error conditions. Even when KLockFile sets mustCloseFd, the dtor's impl > also checks that the lock has been taken, which is only considered true > if LockOK had been returned in our lock function. Instead close() the fd > ourselves unless we make it to LockOK. > > 2. CID 1175555; The standard mis-use of QCache. QCache::insert can, in > theory, delete our object as soon as we insert it into cache, so we have > to check for that. Even ::contains() and ::object() can be risky (the > pointers returned by object() have no lifetime guarantee), but since > this is GUI code I assume it's only used single-threaded and not > re-entrant. Otherwise we'd need even more paranoia... > > > Diffs > ----- > > src/kdecore/klockfile_unix.cpp 67283a5 > src/kdeui/k4style.cpp a1a2ab1 > > Diff: https://git.reviewboard.kde.org/r/126507/diff/ > > > Testing > ------- > > Everything builds and appears to still work, though it's hard to test K4Style > as I'm not sure what uses it right at this point. > > > Thanks, > > Michael Pyne > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel