Review Request 126506: Fix improper destruction of non-virtual KDirModel subclasses

[David Faure]

> On Dec. 25, 2015, 1:58 a.m., Aleix Pol Gonzalez wrote:
> > If making the destructor virtual doesn't break ABI, I'd vote for that...

The node classes are internal, so this wouldn't break ABI indeed.

Hmm, right, this would make "delete node" work everywhere. Michael's fix misses the other place where a node is deleted:

2 * delete dirNode->m_childNodes.takeAt(r); in KDirModelPrivate::_k_slotDeleteItems
1 * in KDirModelPrivate::_k_slotRefreshItems

You're right, I withdraw my Ship It, we need a virtual dtor instead.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126506/#review90080
-----------------------------------------------------------


On Dec. 25, 2015, 12:12 a.m., Michael Pyne wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126506/
> -----------------------------------------------------------
> 
> (Updated Dec. 25, 2015, 12:12 a.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> Noted by Coverity (CID 1019869), and could result in a set of
> partially-destructed objects. Which, in this case, would probably leak
> memory in the event of multiple levels of filesystem hierarchy from the
> root KDirModel, but wouldn't cause any worse problems than that.
> 
> The 'proper' fix would be to add a virtual dtor at the base class but I
> didn't want to add a vtable just for this, so I mirrored the rest of the
> code and utilized the fact that item().isDir() is true for all derived
> classes.
> 
> 
> Diffs
> -----
> 
>   src/widgets/kdirmodel.cpp af56a06 
> 
> Diff: https://git.reviewboard.kde.org/r/126506/diff/
> 
> 
> Testing
> -------
> 
> Builds, kdirmodeltest passes.
> 
> 
> Thanks,
> 
> Michael Pyne
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20151225/32d390eb/attachment.html>