Fix crash-prone weak pointers, atomic save, and misc cleanup

- Guard all toStrongRef() calls with null checks to prevent crashes
- Save project files atomically (write to .tmp, then rename)
- Fix macOS CI workflow typo that broke test discovery
- Remove duplicate main.qrc entry in mapmap.pro
- Upgrade C++ standard from c++11 to c++17
- Replace NULL with nullptr throughout our code
- Remove stale FIXME comment and duplicate #include

Author: aalex

.github/workflows/macos-build.yml

@@ -31,7 +31,7 @@ jobs:
 
     - name: Build and run tests
       run: |
-        export PATH="($brew --prefix=qt)/bin:$PATH"
+        export PATH="$(brew --prefix qt)/bin:$PATH"
         cd tests/
         qmake6 tests.pro
         make -j$(sysctl -n hw.logicalcpu)

mapmap.pro

@@ -1,6 +1,6 @@
 CONFIG += qt
 CONFIG += debug
-CONFIG += c++11
+CONFIG += c++17
 
 TEMPLATE = app
 
@@ -27,8 +27,7 @@ RESOURCES = \
     main.qrc \
     translations/translation.qrc \
     docs/documentation.qrc \
-    resources/interface.qrc \
-    main.qrc # Main resource file
+    resources/interface.qrc
 
 # Manage lrelease (for translations)
 isEmpty(QMAKE_LRELEASE) {

src/app/main.cpp

@@ -14,7 +14,6 @@
 #include "MetaObjectRegistry.h"
 
 #include <stdlib.h>
-#include <iostream>
 
 MM_USE_NAMESPACE
 

src/control/OscInterface.h

@@ -41,7 +41,6 @@ class OscInterface {
 public:
   typedef QSharedPointer<OscInterface> ptr;
 
-  // FIXME: change listen_port to a int
   OscInterface(int listen_port);
   ~OscInterface();
 

src/core/Commands.cpp

@@ -95,12 +95,16 @@ TransformShapeCommand::TransformShapeCommand(MapperGLCanvas* canvas, TransformSh
   _option = option;
 
   // Clone shape before applying transform.
-  _originalShape.reset(_shape.toStrongRef()->clone());
+  auto strong = _shape.toStrongRef();
+  if (strong)
+    _originalShape.reset(strong->clone());
 }
 
 void TransformShapeCommand::undo() {
   // Copy back shape.
-  _shape.toStrongRef()->copyFrom(*_originalShape);
+  auto strong = _shape.toStrongRef();
+  if (!strong) return;
+  strong->copyFrom(*_originalShape);
 
   // Update everything.
   _canvas->currentShapeWasChanged();

src/gui/ConsoleWindow.cpp

@@ -22,7 +22,7 @@
 
 namespace mmp {
 
-ConsoleWindow* ConsoleWindow::instance = NULL;
+ConsoleWindow* ConsoleWindow::instance = nullptr;
 
 ConsoleWindow::ConsoleWindow(QWidget *parent) : QMainWindow(parent)
 {
@@ -142,7 +142,7 @@ void ConsoleWindow::kill()
 {
   if (instance) {
     delete instance;
-    instance = NULL;
+    instance = nullptr;
   }
 }
 

src/gui/LayerGui.cpp

@@ -25,8 +25,8 @@ namespace mmp {
 
 LayerGui::LayerGui(Layer::ptr layer)
   : _layer(layer),
-    _graphicsItem(NULL),
-    _inputGraphicsItem(NULL)
+    _graphicsItem(nullptr),
+    _inputGraphicsItem(nullptr)
 {
   outputShape = layer->getShape();
   Q_CHECK_PTR(outputShape);
@@ -275,7 +275,7 @@ EllipseColorLayerGui::EllipseColorLayerGui(Layer::ptr layer) : ColorLayerGui(lay
 
 TextureLayerGui::TextureLayerGui(QSharedPointer<TextureLayer> mapping)
   : LayerGui(mapping),
-    _meshItem(NULL)
+    _meshItem(nullptr)
 {
   // Assign members pointers.
   textureLayer = qSharedPointerCast<TextureLayer>(_layer);

src/gui/MainWindow.cpp

@@ -54,7 +54,7 @@ MainWindow::MainWindow()
   // TODO: not sure we need this anymore since we have NULL_UID
   _hasCurrentSource = false;
   _hasCurrentLayer = false;
-  currentSelectedItem = NULL;
+  currentSelectedItem = nullptr;
 
   // Frames per second.
   _framesPerSecond = (-1);
@@ -2225,7 +2225,7 @@ void MainWindow::startFullScreen()
 
 void MainWindow::createMenus()
 {
-  QMenuBar *menuBar = NULL;
+  QMenuBar *menuBar = nullptr;
 
 #ifdef __MACOSX_CORE__
   menuBar = new QMenuBar(0);
@@ -2624,7 +2624,9 @@ bool MainWindow::loadFile(const QString &fileName)
 
 bool MainWindow::saveFile(const QString &fileName)
 {
-  QFile file(fileName);
+  // Write to a temporary file first, then rename for atomic save.
+  QString tmpFileName = fileName + ".tmp";
+  QFile file(tmpFileName);
   if (! file.open(QFile::WriteOnly | QFile::Text))
   {
     QMessageBox::warning(this, tr("Error saving mapping project"),
@@ -2637,12 +2639,26 @@ bool MainWindow::saveFile(const QString &fileName)
   ProjectWriter writer(this);
   if (writer.writeFile(&file))
   {
+    file.close();
+    // Remove original and rename temp file over it.
+    QFile::remove(fileName);
+    if (!QFile::rename(tmpFileName, fileName))
+    {
+      QMessageBox::warning(this, tr("Error saving mapping project"),
+                           tr("Cannot rename temporary file to %1.")
+                           .arg(fileName));
+      return false;
+    }
     setCurrentFile(fileName);
     statusBar()->showMessage(tr("File saved"), 2000);
     return true;
   }
   else
+  {
+    file.close();
+    QFile::remove(tmpFileName);
     return false;
+  }
 }
 
 void MainWindow::setCurrentFile(const QString &fileName)
@@ -2972,7 +2988,7 @@ void MainWindow::addSourceItem(uid sourceId, const QIcon& icon, const QString& n
 
 void MainWindow::updateSourceItem(uid sourceId, const QIcon& icon, const QString& name) {
   QListWidgetItem* item = getItemFromId(*sourceList, sourceId);
-  if (item == NULL) {
+  if (item == nullptr) {
     // FIXME there was an assert that seemed to make MapMap crash, here.
     return;
   }
@@ -3196,7 +3212,7 @@ void MainWindow::removeSourceItem(uid sourceId)
   Q_ASSERT( row >= 0 );
   QListWidgetItem* item = sourceList->takeItem(row);
   if (item == currentSelectedItem)
-    currentSelectedItem = NULL;
+    currentSelectedItem = nullptr;
   delete item;
 
   // Update list.
@@ -3601,7 +3617,7 @@ QListWidgetItem* MainWindow::getItemFromId(const QListWidget& list, uid id) {
   if (row >= 0)
     return list.item( row );
   else
-    return NULL;
+    return nullptr;
 }
 
 int MainWindow::getItemRowFromId(const QListWidget& list, uid id)

src/gui/ShapeGraphicsItem.cpp

@@ -136,8 +136,9 @@ PolygonColorGraphicsItem::PolygonColorGraphicsItem(Layer::ptr mapping, bool outp
 QPainterPath PolygonColorGraphicsItem::shape() const
 {
   QPainterPath path;
-  Polygon* poly = static_cast<Polygon*>(_shape.toStrongRef().data());
-  Q_ASSERT(poly);
+  auto strong = _shape.toStrongRef();
+  if (!strong) return path;
+  Polygon* poly = static_cast<Polygon*>(strong.data());
   path.addPolygon(poly->toPolygon());
   return mapFromScene(path);
 }
@@ -146,8 +147,9 @@ void PolygonColorGraphicsItem::_doPaint(QPainter *painter,
                                         const QStyleOptionGraphicsItem *option)
 {
   Q_UNUSED(option);
-  Polygon* poly = static_cast<Polygon*>(_shape.toStrongRef().data());
-  Q_ASSERT(poly);
+  auto strong = _shape.toStrongRef();
+  if (!strong) return;
+  Polygon* poly = static_cast<Polygon*>(strong.data());
   painter->drawPolygon(mapFromScene(poly->toPolygon()));
 }
 
@@ -162,7 +164,9 @@ void MeshColorGraphicsItem::_doPaint(QPainter *painter,
 {
   Q_UNUSED(option);
 
-  Mesh* mesh = static_cast<Mesh*>(_shape.toStrongRef().data());
+  auto strong = _shape.toStrongRef();
+  if (!strong) return;
+  Mesh* mesh = static_cast<Mesh*>(strong.data());
   QVector<QVector<Quad::ptr> > quads = mesh->getQuads2d();
 
   // Go through the mesh quad by quad.
@@ -186,8 +190,9 @@ QPainterPath EllipseColorGraphicsItem::shape() const
 {
   // Create path for ellipse.
   QPainterPath path;
-  Ellipse* ellipse = static_cast<Ellipse*>(_shape.toStrongRef().data());
-  Q_ASSERT(ellipse);
+  auto strong = _shape.toStrongRef();
+  if (!strong) return path;
+  Ellipse* ellipse = static_cast<Ellipse*>(strong.data());
   QTransform transform;
   transform.translate(ellipse->getCenter().x(), ellipse->getCenter().y());
   transform.rotate(ellipse->getRotation());
@@ -313,14 +318,17 @@ void TextureGraphicsItem::_postPaint(QPainter* painter,
 
 QSharedPointer<Texture> TextureGraphicsItem::_getTexture()
 {
-  return qSharedPointerCast<Texture>(_textureMapping.toStrongRef()->getSource());
+  auto strong = _textureMapping.toStrongRef();
+  if (!strong) return QSharedPointer<Texture>();
+  return qSharedPointerCast<Texture>(strong->getSource());
 }
 
 QPainterPath PolygonTextureGraphicsItem::shape() const
 {
   QPainterPath path;
-  Polygon* poly = static_cast<Polygon*>(_shape.toStrongRef().data());
-  Q_ASSERT(poly);
+  auto strong = _shape.toStrongRef();
+  if (!strong) return path;
+  Polygon* poly = static_cast<Polygon*>(strong.data());
   path.addPolygon(poly->toPolygon());
   return mapFromScene(path);
 }
@@ -335,6 +343,7 @@ void TriangleTextureGraphicsItem::_doDrawOutput(QPainter* painter)
   if (isOutput())
   {
     MShape::ptr inputShape = _inputShape.toStrongRef();
+    if (!inputShape) return;
     glBegin(GL_TRIANGLES);
     {
       for (int i=0; i<inputShape->nVertices(); i++)
@@ -563,8 +572,9 @@ QPainterPath EllipseTextureGraphicsItem::shape() const
 {
   // Create path for ellipse.
   QPainterPath path;
-  Ellipse* ellipse = static_cast<Ellipse*>(_shape.toStrongRef().data());
-  Q_ASSERT(ellipse);
+  auto strong = _shape.toStrongRef();
+  if (!strong) return path;
+  Ellipse* ellipse = static_cast<Ellipse*>(strong.data());
   QTransform transform;
   transform.translate(ellipse->getCenter().x(), ellipse->getCenter().y());
   transform.rotate(ellipse->getRotation());