D16663: [import] Fix leaking AbstractFileFilters
Christoph Roick
noreply at phabricator.kde.org
Wed Dec 5 23:16:39 GMT 2018
croick marked an inline comment as done.
croick added inline comments.
INLINE COMMENTS
> asemke wrote in LiveDataSource.cpp:252
> this needs only be done if m_filter != nulltpr,
Actually nothing happens when deleting a nullptr (https://en.cppreference.com/w/cpp/language/delete). The check would be redundant. For newer versions of C++ this assumption seems to change a little, in case a non-standard deallocator is used – which we don't, the standard deallocator just does nothing with a nullptr.
> asemke wrote in ImportFileWidget.cpp:595
> I think the following would also to the job completely without any smart pointers and static casts:
>
> if (m_currentFilter)
> delete m_currentFilter;
>
> auto filter = new AsciiFilter();
> filter->set*();
> m_currentFilter = filter;
The point of keeping the filter is not just a correct cleanup, but also to omit reparsing the file that is to be imported. `currentFileFilter()` gets called a lot and makes the UI unresponsive otherwise. (Which was my initial reason for this patch.)
I could switch to a raw pointer still, but that would be inconsistent with the pointers for the options widgets.
REPOSITORY
R262 LabPlot
REVISION DETAIL
https://phabricator.kde.org/D16663
To: croick, #labplot, asemke
Cc: asemke, kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20181205/90abecf6/attachment.html>
More information about the kde-edu
mailing list