D15295: [konsole]: proper fix for a crash-on-exit

[Martin Tobias Holmedahl Sandsmark]

sandsmark accepted this revision.
sandsmark added a comment.
This revision is now accepted and ready to land.


  The issues are just cosmetic, didn't intend to block this.
  
  But Hindenburg should probably give a go-ahead as well.

INLINE COMMENTS

> rjvbb wrote in ViewSplitter.cpp:46
> I simply haven't adapted yet to the fact that there's now a "normal for" equivalent of Qt's equally "normal foreach"...
> 
> If you prefer I can use an explicit type but I think this is as good a place to be lazy as any. Besides, in any self-respecting IDE you can hover the mouse cursor over either the variable or `unregisterContainer()` and a popup will tell you what type the variable (must) be ;)

I don't always use an IDE when reading code, and I (almost) never use the mouse when writing code. But again, it's my own personal preference, Hindenburg uses auto a lot all over the place.

And looking around, seems like foreach isn't used extensively either, so they all can probably be converted automatically en masse later.

> rjvbb wrote in ViewSplitter.h:185
> I'm not certain I get this. Put `_containers` in a QPointer or make it a `QList< QPointer<TabbedViewContainer *> >`?
> 
> The former is impossible AFAIK (QList doesn't inherit QObject), while the latter isn't relevant because the crash was caused by accessing a stale `_containers` instance (i.e. a member variable of a stale `ViewSplitter` instance).

wasn't thinking about it being an issue now, but rather in the future.

a QPointer here shouldn't impact performance anyways.

REPOSITORY
  R319 Konsole

REVISION DETAIL
  https://phabricator.kde.org/D15295

To: rjvbb, #konsole, hindenburg, sandsmark
Cc: sandsmark, anthonyfieroni, tcanabrava, konsole-devel, #kde_applications, maciejn, thsurrel, ngraham, maximilianocuria, hindenburg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20190208/3db0f831/attachment-0001.html>